From eb295d3f6f2fb1477e585c1a36a2de01072ef18f Mon Sep 17 00:00:00 2001 From: Harlan GitHub Agent Date: Sun, 6 Sep 2026 17:48:33 +1000 Subject: [PATCH] fix: resolve issue #117 --- server/plugins/sentry.ts | 12 +++- shared/sentry.ts | 20 ++++++ test/unit/sentry-cache-write-filter.test.ts | 76 +++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/unit/sentry-cache-write-filter.test.ts diff --git a/server/plugins/sentry.ts b/server/plugins/sentry.ts index b507b178..000d0765 100644 --- a/server/plugins/sentry.ts +++ b/server/plugins/sentry.ts @@ -1,5 +1,5 @@ import { sentryCloudflareNitroPlugin } from '@sentry/nuxt/module/plugins' -import { createSentryDataCollection } from '../../shared/sentry' +import { createSentryDataCollection, isBestEffortCacheWriteError } from '../../shared/sentry' export default defineNitroPlugin((nitroApp) => { const { sentry } = useRuntimeConfig() @@ -14,5 +14,15 @@ export default defineNitroPlugin((nitroApp) => { release: sentry.release || undefined, tracesSampleRate: sentry.tracesSampleRate, dataCollection: createSentryDataCollection(), + // Nitro's route cache catches its own KV write failures and forwards the + // caught error here as unhandled (SKILLD-17). That is the same failure + // class `writeCache` records as a wide event, so drop the duplicate and + // keep the wide event as the visibility path. + beforeSend(event, hint) { + const values = event.exception?.values ?? [] + if (isBestEffortCacheWriteError(hint.originalException) || values.some(isBestEffortCacheWriteError)) + return null + return event + }, })(nitroApp) }) diff --git a/shared/sentry.ts b/shared/sentry.ts index 7b0ecf86..1ee08bec 100644 --- a/shared/sentry.ts +++ b/shared/sentry.ts @@ -36,6 +36,26 @@ export function sentryRelease(): string | undefined { return process.env.SENTRY_RELEASE || process.env.GITHUB_SHA || undefined } +/** + * The message prefix Workers KV rejects every cache write with + * (`KV PUT failed: 429 Too Many Requests`, Sentry SKILLD-17). Nitro's route + * cache treats the rejection as best-effort: it catches it, logs it, and still + * serves the response, then forwards the caught error to Sentry as unhandled. + * `writeCache` (shared/server/cache.ts) holds the same line for the project's + * own cache writes and records the failure as a wide event instead, so the + * forwarded error is a duplicate of a failure class already handled. The + * server Sentry config uses this predicate to drop exactly that signature in + * `beforeSend`; every other error still reaches Sentry. + */ +export function isBestEffortCacheWriteError(error: unknown): boolean { + const message = error instanceof Error + ? error.message + : typeof error === 'object' && error !== null + ? (error as { message?: unknown }).message ?? (error as { value?: unknown }).value + : undefined + return typeof message === 'string' && message.startsWith('KV PUT failed:') +} + export function createSentryDataCollection() { return { userInfo: false, diff --git a/test/unit/sentry-cache-write-filter.test.ts b/test/unit/sentry-cache-write-filter.test.ts new file mode 100644 index 00000000..9579c02b --- /dev/null +++ b/test/unit/sentry-cache-write-filter.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { isBestEffortCacheWriteError } from '../../shared/sentry' + +describe('isBestEffortCacheWriteError', () => { + it('matches the SKILLD-17 rate-limited KV write signature', () => { + expect(isBestEffortCacheWriteError(new Error('KV PUT failed: 429 Too Many Requests'))).toBe(true) + }) + + it('matches the Sentry-serialized exception, whose message arrives as value', () => { + expect(isBestEffortCacheWriteError({ + type: 'Error', + value: 'KV PUT failed: 429 Too Many Requests', + mechanism: { handled: false, type: 'auto.function.nuxt.nitro' }, + })).toBe(true) + }) + + it('ignores KV read failures, which are a different failure class', () => { + expect(isBestEffortCacheWriteError(new Error('KV GET failed: 500 Internal Server Error'))).toBe(false) + }) + + it('ignores unrelated errors and values that are not errors', () => { + expect(isBestEffortCacheWriteError(new Error('D1_ERROR: D1 DB is overloaded.'))).toBe(false) + expect(isBestEffortCacheWriteError('KV PUT failed: 429 Too Many Requests')).toBe(false) + expect(isBestEffortCacheWriteError(undefined)).toBe(false) + }) +}) + +const { sentryCloudflareNitroPlugin } = vi.hoisted(() => ({ + sentryCloudflareNitroPlugin: vi.fn(), +})) + +vi.mock('@sentry/nuxt/module/plugins', () => ({ sentryCloudflareNitroPlugin })) + +interface SentryEventFixture { + exception?: { values?: Array<{ type?: string, value?: string }> } +} + +describe('sentry beforeSend cache-write filter', () => { + let beforeSend: (event: SentryEventFixture, hint: { originalException?: unknown }) => unknown + + beforeEach(async () => { + vi.stubGlobal('defineNitroPlugin', (plugin: (nitroApp: unknown) => void) => plugin) + sentryCloudflareNitroPlugin.mockClear() + sentryCloudflareNitroPlugin.mockReturnValue(vi.fn()) + const config = (useNuxtApp().$config as { sentry: Record }).sentry + config.enabled = true + config.dsn = 'https://examplePublicKey@o0.ingest.sentry.io/0' + const registerPlugin = (await import('../../server/plugins/sentry')).default as + (nitroApp: unknown) => void + registerPlugin({}) + beforeSend = sentryCloudflareNitroPlugin.mock.calls[0]![0].beforeSend + }) + + it('drops the route-cache KV write failure Nitro already caught', () => { + const event: SentryEventFixture = { + exception: { values: [{ type: 'Error', value: 'KV PUT failed: 429 Too Many Requests' }] }, + } + expect(beforeSend(event, { originalException: new Error('KV PUT failed: 429 Too Many Requests') })) + .toBeNull() + }) + + it('drops on the serialized exception alone, when the original error is absent', () => { + const event: SentryEventFixture = { + exception: { values: [{ type: 'Error', value: 'KV PUT failed: 429 Too Many Requests' }] }, + } + expect(beforeSend(event, {})).toBeNull() + }) + + it('keeps every other event', () => { + const event: SentryEventFixture = { + exception: { values: [{ type: 'Error', value: 'D1_ERROR: D1 DB is overloaded.' }] }, + } + expect(beforeSend(event, { originalException: new Error('D1_ERROR: D1 DB is overloaded.') })) + .toBe(event) + }) +})