From e31d73b3dfc991383bdd5c5adb4d99db855121cc Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sat, 30 May 2026 15:37:38 +0200
Subject: [PATCH] fix(keyvalue): treat undecodable cached values as a cache
 miss

A GetWithValue deserialization error in RememberFor was returned as fatal.
On a Redis upgrade the metrics counters live under the same keys as before
but were stored as plain int64, so the first decode into the new envelope
would fail and the metric would break permanently. Treat such errors as a
miss and recompute/overwrite so the cache self-heals.
---
 pkg/modules/keyvalue/keyvalue.go      | 10 +++++-----
 pkg/modules/keyvalue/keyvalue_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/pkg/modules/keyvalue/keyvalue.go b/pkg/modules/keyvalue/keyvalue.go
index c71e00063..137ea8f62 100644
--- a/pkg/modules/keyvalue/keyvalue.go
+++ b/pkg/modules/keyvalue/keyvalue.go
@@ -155,14 +155,14 @@ type expiringValue[T any] struct {
 // older than ttl. On a miss or once expired, it executes fn, caches the result for
 // ttl and returns it. If fn returns an error, nothing is cached.
 // T must be a concrete (non-pointer) type.
+//
+// A value that cannot be deserialized into the expected type is treated as a cache
+// miss and overwritten, so the cache self-heals across upgrades that change what a key
+// stores (e.g. a key that previously held a plain int64 in Redis).
 func RememberFor[T any](key string, ttl time.Duration, fn func() (T, error)) (T, error) {
 	var cached expiringValue[T]
 	exists, err := GetWithValue(key, &cached)
-	if err != nil {
-		var zero T
-		return zero, err
-	}
-	if exists && time.Now().Before(cached.ExpiresAt) {
+	if err == nil && exists && time.Now().Before(cached.ExpiresAt) {
 		return cached.Value, nil
 	}
 
diff --git a/pkg/modules/keyvalue/keyvalue_test.go b/pkg/modules/keyvalue/keyvalue_test.go
index b95fc2ec5..ca32d8aec 100644
--- a/pkg/modules/keyvalue/keyvalue_test.go
+++ b/pkg/modules/keyvalue/keyvalue_test.go
@@ -132,3 +132,28 @@ func TestRememberForErrorDoesNotStore(t *testing.T) {
 	require.NoError(t, err2)
 	assert.False(t, exists)
 }
+
+// getWithValueErrorStore simulates a backend that cannot deserialize an existing value
+// into the requested type, e.g. a key that held a plain int64 before the cache started
+// storing a struct (the pre-refactor metrics counters in Redis).
+type getWithValueErrorStore struct {
+	*memory.Storage
+}
+
+func (s *getWithValueErrorStore) GetWithValue(string, interface{}) (bool, error) {
+	return false, errors.New("decode error")
+}
+
+func TestRememberForRecomputesWhenStoredValueCannotBeDeserialized(t *testing.T) {
+	store = &getWithValueErrorStore{memory.NewStorage()}
+
+	called := 0
+	val, err := RememberFor("foo", time.Hour, func() (int64, error) {
+		called++
+		return 42, nil
+	})
+
+	require.NoError(t, err)
+	assert.Equal(t, int64(42), val)
+	assert.Equal(t, 1, called)
+}