Skip to content

fix(cache): check for any type of redisNil wrapped error

Suleimi Ahmed requested to merge 875-assert-redis-nil-returned-from-cache into master

Related to #875 (closed)

Context

Updating the gocache package unearthed a compatibility issue in the new version of the package that we've adopted (here).

In the prior version of gocache that we used (i.e v2) on Get it triggered this:

func (s *RedisStore) Get(ctx context.Context, key interface{}) (interface{}, error) {
	return s.client.Get(ctx, key.(string)).Result()
}

on the new version we migrated to (i.e v4) to it seems to do this:

func (s *RedisStore) Get(ctx context.Context, key any) (any, error) {
	object, err := s.client.Get(ctx, key.(string)).Result()
	if err == redis.Nil {
		return nil, lib_store.NotFoundWithCause(err)
	}
	return object, err
}

Why is this important?

We rely on an explicit check for if redis.Nil in a few of our code paths (e.g here). So if we no longer get redis.Nil where we assumed we would get it in and we now get lib_store.NotFoundWithCause(err) then we will not be leveraging the cache as we presumed.

Whats in the MR?

Rather than checking for the redis.Nil we checked for any sort of wrapped redis.Nil ; using errors.Is(err, redis.Nil) rather than err == redis.Nil

Merge request reports