fix(nitro): Address review feedback for unstorage instrumentation

- hasItem: use boolean result directly instead of generic isCacheHit
- isCacheHit: handle both string and already-deserialized cache values
- cache tests: replace silent if-guards with hard assertions

Author: logaretm

dev-packages/e2e-tests/test-applications/nitro-3/tests/cache.test.ts

@@ -38,14 +38,13 @@ test.describe('Cache Instrumentation', () => {
         span.data[SEMANTIC_ATTRIBUTE_CACHE_KEY].includes('user:123') &&
         !span.data?.[SEMANTIC_ATTRIBUTE_CACHE_HIT],
     );
-    if (cacheMissSpan) {
-      expect(cacheMissSpan.data).toMatchObject({
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item',
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
-        [SEMANTIC_ATTRIBUTE_CACHE_HIT]: false,
-        'db.operation.name': 'getItem',
-      });
-    }
+    expect(cacheMissSpan).toBeDefined();
+    expect(cacheMissSpan?.data).toMatchObject({
+      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item',
+      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
+      [SEMANTIC_ATTRIBUTE_CACHE_HIT]: false,
+      'db.operation.name': 'getItem',
+    });
 
     // Find cache hit (second call to getCachedUser('123'))
     const cacheHitSpan = getItemSpans.find(
@@ -54,14 +53,13 @@ test.describe('Cache Instrumentation', () => {
         span.data[SEMANTIC_ATTRIBUTE_CACHE_KEY].includes('user:123') &&
         span.data?.[SEMANTIC_ATTRIBUTE_CACHE_HIT],
     );
-    if (cacheHitSpan) {
-      expect(cacheHitSpan.data).toMatchObject({
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item',
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
-        [SEMANTIC_ATTRIBUTE_CACHE_HIT]: true,
-        'db.operation.name': 'getItem',
-      });
-    }
+    expect(cacheHitSpan).toBeDefined();
+    expect(cacheHitSpan?.data).toMatchObject({
+      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item',
+      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
+      [SEMANTIC_ATTRIBUTE_CACHE_HIT]: true,
+      'db.operation.name': 'getItem',
+    });
 
     // setItem spans for cachedFunction - when cache miss occurs, value is set
     const setItemSpans = findSpansByOp('cache.set_item');
@@ -72,13 +70,12 @@ test.describe('Cache Instrumentation', () => {
         typeof span.data?.[SEMANTIC_ATTRIBUTE_CACHE_KEY] === 'string' &&
         span.data[SEMANTIC_ATTRIBUTE_CACHE_KEY].includes('user:123'),
     );
-    if (cacheSetSpan) {
-      expect(cacheSetSpan.data).toMatchObject({
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.set_item',
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
-        'db.operation.name': 'setItem',
-      });
-    }
+    expect(cacheSetSpan).toBeDefined();
+    expect(cacheSetSpan?.data).toMatchObject({
+      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.set_item',
+      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.cache.nitro',
+      'db.operation.name': 'setItem',
+    });
 
     // Spans for different cached functions
     const dataKeySpans = getItemSpans.filter(

packages/nitro/src/runtime/hooks/captureStorageEvents.ts

@@ -79,7 +79,8 @@ function setupStorageTracingChannel(operation: TracedOperation): void {
   channel.subscribe({
     asyncEnd(data: TracingChannelContextWithSpan<TraceContext & { result?: unknown }>) {
       if (data._sentrySpan && CACHE_HIT_OPERATIONS.has(operation)) {
-        data._sentrySpan.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, isCacheHit(data.keys?.[0], data.result));
+        const hit = operation === 'hasItem' ? Boolean(data.result) : isCacheHit(data.keys?.[0], data.result);
+        data._sentrySpan.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, hit);
       }
 
       data._sentrySpan?.setStatus({ code: SPAN_STATUS_OK });
@@ -126,7 +127,9 @@ function isCacheHit(key: unknown, value: unknown): boolean {
       return !isEmpty;
     }
 
-    return validateCacheEntry(key, JSON.parse(String(value)) as CacheEntry);
+    const entry = typeof value === 'string' ? (JSON.parse(value) as CacheEntry) : (value as CacheEntry);
+
+    return validateCacheEntry(key, entry);
   } catch {
     return false;
   }