Don't bother with error channels, nobody is going to use them

Author: Thomas Stromberg

persistent.go

@@ -3,6 +3,7 @@ package bdcache
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"time"
 )
 
@@ -181,9 +182,9 @@ func (c *PersistentCache[K, V]) Set(ctx context.Context, key K, value V, ttl ...
 // SetAsync stores a value in the cache, handling persistence asynchronously.
 // If no TTL is provided, the default TTL is used.
 // Key validation and in-memory caching happen synchronously.
-// Returns a channel that will receive any persistence error (or be closed on success).
+// Persistence errors are logged but not returned (fire-and-forget).
 // Returns an error only for validation failures (e.g., invalid key format).
-func (c *PersistentCache[K, V]) SetAsync(ctx context.Context, key K, value V, ttl ...time.Duration) (<-chan error, error) {
+func (c *PersistentCache[K, V]) SetAsync(ctx context.Context, key K, value V, ttl ...time.Duration) error {
 	var t time.Duration
 	if len(ttl) > 0 {
 		t = ttl[0]
@@ -192,24 +193,23 @@ func (c *PersistentCache[K, V]) SetAsync(ctx context.Context, key K, value V, tt
 
 	// Validate key early (synchronous)
 	if err := c.Store.ValidateKey(key); err != nil {
-		return nil, err
+		return err
 	}
 
 	// ALWAYS update memory first - reliability guarantee (synchronous)
 	c.memory.setToMemory(key, value, timeToNano(expiry))
 
-	// Update persistence asynchronously
-	errCh := make(chan error, 1)
+	// Update persistence asynchronously (fire-and-forget)
+	//nolint:contextcheck // Intentionally detached - persistence should complete even if caller cancels
 	go func() {
-		defer close(errCh)
-		storeCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+		storeCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 		defer cancel()
 		if err := c.Store.Store(storeCtx, key, value, expiry); err != nil {
-			errCh <- fmt.Errorf("async persistence: %w", err)
+			slog.Error("async persistence failed", "key", key, "error", err)
 		}
 	}()
 
-	return errCh, nil
+	return nil
 }
 
 // Delete removes a value from the cache.

persistent_test.go

@@ -303,8 +303,7 @@ func TestPersistentCache_SetAsync(t *testing.T) {
 	defer func() { _ = cache.Close() }() //nolint:errcheck // Test cleanup
 
 	// SetAsync should not block but value should be available immediately
-	errCh, err := cache.SetAsync(ctx, "key1", 42, 0)
-	if err != nil {
+	if err := cache.SetAsync(ctx, "key1", 42, 0); err != nil {
 		t.Fatalf("SetAsync: %v", err)
 	}
 
@@ -317,10 +316,8 @@ func TestPersistentCache_SetAsync(t *testing.T) {
 		t.Error("key1 should be available immediately after SetAsync")
 	}
 
-	// Wait for async persistence to complete
-	if err := <-errCh; err != nil {
-		t.Fatalf("SetAsync persistence: %v", err)
-	}
+	// Give async persistence time to complete
+	time.Sleep(50 * time.Millisecond)
 
 	// Should also be persisted
 	val, _, found, err = store.Load(ctx, "key1")
@@ -377,10 +374,9 @@ func TestPersistentCache_Errors(t *testing.T) {
 		t.Error("key1 should be in memory even though persistence failed")
 	}
 
-	// SetAsync returns error channel for persistence errors
+	// SetAsync logs persistence errors but doesn't return them
 	store.failSet = true
-	errCh, err := cache.SetAsync(ctx, "key3", 300, 0)
-	if err != nil {
+	if err := cache.SetAsync(ctx, "key3", 300, 0); err != nil {
 		t.Fatalf("SetAsync should not fail synchronously: %v", err)
 	}
 
@@ -393,10 +389,8 @@ func TestPersistentCache_Errors(t *testing.T) {
 		t.Error("key3 should be in memory after SetAsync")
 	}
 
-	// Persistence error should come through the channel
-	if err := <-errCh; err == nil {
-		t.Error("SetAsync should report persistence error through channel")
-	}
+	// Give async persistence time to attempt (and fail with logged error)
+	time.Sleep(50 * time.Millisecond)
 
 	// Get should work from memory even if persistence fails
 	store.failGet = true
@@ -868,25 +862,15 @@ func TestPersistentCache_SetAsync_VariadicTTL(t *testing.T) {
 	defer func() { _ = cache.Close() }() //nolint:errcheck // Test cleanup
 
 	// SetAsync without TTL - uses default
-	errCh1, err := cache.SetAsync(ctx, "async-default", 1)
-	if err != nil {
+	if err := cache.SetAsync(ctx, "async-default", 1); err != nil {
 		t.Fatalf("SetAsync: %v", err)
 	}
 
 	// SetAsync with explicit TTL
-	errCh2, err := cache.SetAsync(ctx, "async-explicit", 2, 5*time.Minute)
-	if err != nil {
+	if err := cache.SetAsync(ctx, "async-explicit", 2, 5*time.Minute); err != nil {
 		t.Fatalf("SetAsync: %v", err)
 	}
 
-	// Wait for persistence
-	if err := <-errCh1; err != nil {
-		t.Fatalf("SetAsync persistence: %v", err)
-	}
-	if err := <-errCh2; err != nil {
-		t.Fatalf("SetAsync persistence: %v", err)
-	}
-
 	// Both should be in memory immediately
 	_, found, err := cache.Get(ctx, "async-default")
 	if err != nil {