Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigCache implementation does not return an error of type *store.NotFound when the key is not found #224

Open
butaca opened this issue Sep 7, 2023 · 2 comments

Comments

@butaca
Copy link

butaca commented Sep 7, 2023

I'm trying to check if a key exists in the cache. Since there is not a CacheInterface.Exists, I have to rely on CacheInterface.Get to perform the check. The Redis implementation correctly returns *store.NotFound when the key is not found, but the BigCache one does not.

Steps for Reproduction

  1. Set up BigCache:
bigcacheClient, _ := bigcache.New(ctx, bigcache.DefaultConfig(5 * time.Minute))
bigcacheStore := bigcache_store.NewBigcache(bigcacheClient)
cacheManager := cache.New[[]byte](bigcacheStore)
  1. Check the error type:
_, err := bigCacheCache.Get(ctx, "key")
_, ok := err.(*store.NotFound)
fmt.Printf("BigCache Type is *store.NotFound: %v\n", ok)
  1. See that is not of type *store.NotFound
  2. Perform the previous steps with Redis and see how the error is of type *store.NotFound

Expected behavior:

Returned error is of type *store.NotFound

Actual behavior:

Returned error is of not of type *store.NotFound

Platforms:

macOS and dockerized Linux from scratch

Versions:

gocache v4.1.3 and v4.1.4
go 1.21
BigCache store v4.1.2 and v4.2.0
BigCache v3.1.0
Redis store v4.2.0
Redis client v9.0.5
Redis server 7.0.12

@ShivamKumar2002
Copy link

This seems like a trivial fix, just need to handle bigcache.ErrEntryNotFound in Get method. However, many existing library usages may be dependent on current behavior which returns error directly. So fixing this may cause issues for some users.

@Fire-Dragon-DoL
Copy link

Fire-Dragon-DoL commented Aug 28, 2024

This seems like a trivial fix, just need to handle bigcache.ErrEntryNotFound in Get method. However, many existing library usages may be dependent on current behavior which returns error directly. So fixing this may cause issues for some users.

The two errors could be joined so that errors.Is would be true for both.
I'm thinking in the current situation, this also breaks loadable cache

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants