mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 01:15:03 +03:00
feat: sample non-crash sentry errors at one percent (#50339)
## Problem Browser Sentry reporting sends ordinary application errors at full volume even though full-page crashes are the highest-priority signal. ## Fix Sample eligible browser errors without `globalErrorBoundary` at 1% across Studio, www, and docs. Keep 100% of eligible errors tagged with `globalErrorBoundary`, preserve consent and existing noise filters, and record the applied rate in `codeSampleRate`. ## How to test - Run `node node_modules/vitest/vitest.mjs run ../../packages/common/sentry.test.ts lib/sentry-capture.test.tsx` from `apps/www`. - Run `node node_modules/vitest/vitest.mjs run lib/sentry-client-options.test.ts` from `apps/studio`. - Expected result: tagged page crashes bypass sampling, ordinary errors use the 1% cutoff, and Studio applies sampling once while preserving its existing filters. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved error reporting reliability by ensuring page-crash errors are captured without sampling. - Non-crash application errors are now sampled at a low rate, with sampling metadata retained for monitoring. - Updated filtering behavior so relevant Studio errors continue to be reported consistently, including errors previously affected by client-side filtering. - Preserved filtering for third-party-only errors that do not represent application failures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
63bedef77f
commit
8984305b1e
5 files changed
+82
-47
No files matched your search
@@ -1,5 +1,5 @@
|
||||
import type { Event as SentryEvent, StackFrame } from '@sentry/react'
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
buildSentryClientOptions,
|
||||
@@ -501,6 +501,7 @@ describe('which errors Studio sends to Sentry', () => {
|
||||
let restoreConsent: () => void
|
||||
|
||||
beforeAll(async () => {
|
||||
vi.spyOn(Math, 'random').mockReturnValue(0)
|
||||
vi.stubEnv('NEXT_PUBLIC_IS_PLATFORM', 'true')
|
||||
vi.resetModules()
|
||||
const { consentState } = await import('common')
|
||||
@@ -515,7 +516,12 @@ describe('which errors Studio sends to Sentry', () => {
|
||||
beforeSend = options.beforeSend
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
vi.restoreAllMocks()
|
||||
restoreConsent?.()
|
||||
vi.unstubAllEnvs()
|
||||
vi.resetModules()
|
||||
@@ -537,6 +543,37 @@ describe('which errors Studio sends to Sentry', () => {
|
||||
}
|
||||
)
|
||||
|
||||
it('samples errors that did not crash the page once', async () => {
|
||||
const event: Parameters<typeof beforeSend>[0] = {
|
||||
type: undefined,
|
||||
exception: {
|
||||
values: [{ value: 'Application error', stacktrace: { frames: [{ filename: 'app.js' }] } }],
|
||||
},
|
||||
}
|
||||
|
||||
expect(await beforeSend(event, {})).toBe(event)
|
||||
expect(Math.random).toHaveBeenCalledOnce()
|
||||
expect(event.tags?.codeSampleRate).toBe('0.01')
|
||||
})
|
||||
|
||||
it('still applies Studio filters to page crashes', async () => {
|
||||
const event: Parameters<typeof beforeSend>[0] = {
|
||||
type: undefined,
|
||||
tags: { globalErrorBoundary: true },
|
||||
exception: {
|
||||
values: [
|
||||
{
|
||||
value: 'captcha.render is not a function',
|
||||
stacktrace: { frames: [{ filename: 'api.js' }] },
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
expect(await beforeSend(event, {})).toBeNull()
|
||||
expect(Math.random).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('drops errors with no code location when they did not crash the page', async () => {
|
||||
expect(
|
||||
await beforeSend({ type: undefined, exception: { values: [{ value: 'No stack' }] } }, {})
|
||||
|
||||
@@ -23,14 +23,6 @@ import { sanitizeArrayOfObjects, sanitizeUrlHashParams } from '@/lib/sanitize'
|
||||
|
||||
type Integration = Parameters<typeof Sentry.addIntegration>[0]
|
||||
|
||||
const DEFAULT_ERROR_SAMPLE_RATE = 1.0
|
||||
const LOW_PRIORITY_ERROR_SAMPLE_RATE = 0.01
|
||||
const CHUNK_LOAD_ERROR_PATTERNS = [
|
||||
/ChunkLoadError/i,
|
||||
/Loading chunk [\d]+ failed/i,
|
||||
/Loading CSS chunk [\d]+ failed/i,
|
||||
]
|
||||
|
||||
// This is a workaround to ignore hCaptcha related errors.
|
||||
function isHCaptchaRelatedError(event: Sentry.Event): boolean {
|
||||
const errors = event.exception?.values ?? []
|
||||
@@ -91,17 +83,6 @@ export function isChallengeExpiredError(error: unknown, event: Sentry.Event): bo
|
||||
return message.includes('challenge-expired')
|
||||
}
|
||||
|
||||
function isChunkLoadError(error: unknown, event: Sentry.Event): boolean {
|
||||
const errorMessage = error instanceof Error ? error.message : ''
|
||||
const eventMessage = event.message || ''
|
||||
const exceptionMessages = event.exception?.values?.map((ex) => ex.value ?? '') ?? []
|
||||
const combinedMessages = [errorMessage, eventMessage, ...exceptionMessages].filter(Boolean)
|
||||
|
||||
return CHUNK_LOAD_ERROR_PATTERNS.some((pattern) =>
|
||||
combinedMessages.some((message) => pattern.test(message))
|
||||
)
|
||||
}
|
||||
|
||||
// Tag errors whose stack trace only contains third-party frames (browser extensions,
|
||||
// injected scripts, etc.). This uses build-time code annotation via the applicationKey
|
||||
// in next.config.ts to reliably distinguish our code from third-party code.
|
||||
@@ -202,29 +183,6 @@ export function buildSentryClientOptions({
|
||||
|
||||
const isErrorBoundaryCrash = isSentryErrorBoundaryCrash(event)
|
||||
|
||||
// Downsample only known high-noise classes; keep all other errors at full rate.
|
||||
const isInvalidUrlEvent = (hint.originalException as any)?.message?.includes(
|
||||
`Failed to construct 'URL': Invalid URL`
|
||||
)
|
||||
const isSessionTimeoutEvent = (hint.originalException as any)?.message?.includes(
|
||||
'Session error detected'
|
||||
)
|
||||
const isChunkLoadFailure = isChunkLoadError(hint.originalException, event)
|
||||
|
||||
const codeSampleRate =
|
||||
isInvalidUrlEvent || isSessionTimeoutEvent || isChunkLoadFailure
|
||||
? LOW_PRIORITY_ERROR_SAMPLE_RATE
|
||||
: DEFAULT_ERROR_SAMPLE_RATE
|
||||
|
||||
if (Math.random() > codeSampleRate) {
|
||||
return null
|
||||
}
|
||||
|
||||
event.tags = {
|
||||
...event.tags,
|
||||
codeSampleRate: codeSampleRate.toString(),
|
||||
}
|
||||
|
||||
if (isHCaptchaRelatedError(event)) {
|
||||
return null
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@ import * as Sentry from '@sentry/nextjs'
|
||||
import { consentState } from 'common/consent-state'
|
||||
import { act } from 'react'
|
||||
import { createRoot } from 'react-dom/client'
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import GlobalError from '../app/global-error'
|
||||
import CustomError from '../pages/_error'
|
||||
@@ -34,10 +34,15 @@ beforeAll(async () => {
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Math, 'random').mockReturnValue(0)
|
||||
envelopes.length = 0
|
||||
consentState.hasConsented = true
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
consentState.hasConsented = false
|
||||
await Sentry.close()
|
||||
|
||||
@@ -1,15 +1,21 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { filterSentryEvent } from './sentry'
|
||||
|
||||
const enabled = { isPlatform: true, hasConsent: true }
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
describe('which errors get sent to Sentry', () => {
|
||||
it.each([undefined, {}, { third_party_code: false }, { third_party_code: 'false' }])(
|
||||
'sends app errors without changing their details: %j',
|
||||
'sends sampled app errors and records the sample rate: %j',
|
||||
(tags) => {
|
||||
vi.spyOn(Math, 'random').mockReturnValue(0)
|
||||
const event = { tags, exception: { values: [{ value: 'Page crashed' }] } }
|
||||
expect(filterSentryEvent(event, enabled)).toBe(event)
|
||||
expect(event.tags).toEqual({ ...tags, codeSampleRate: '0.01' })
|
||||
}
|
||||
)
|
||||
|
||||
@@ -20,14 +26,31 @@ describe('which errors get sent to Sentry', () => {
|
||||
it.each([true, 'true'])(
|
||||
'sends page crashes even when the code location is missing: %s',
|
||||
(tag) => {
|
||||
const random = vi.spyOn(Math, 'random').mockReturnValue(0.99)
|
||||
const event = {
|
||||
tags: { third_party_code: true, globalErrorBoundary: tag },
|
||||
exception: { values: [{ value: 'Page crashed' }] },
|
||||
}
|
||||
expect(filterSentryEvent(event, enabled)).toBe(event)
|
||||
expect(event.tags).toEqual({
|
||||
third_party_code: true,
|
||||
globalErrorBoundary: tag,
|
||||
codeSampleRate: '1',
|
||||
})
|
||||
expect(random).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it.each([
|
||||
[0.0099, true],
|
||||
[0.01, false],
|
||||
])('sends 1%% of errors that did not crash the page: %s', (randomValue, isSent) => {
|
||||
vi.spyOn(Math, 'random').mockReturnValue(randomValue)
|
||||
const event = { tags: {}, exception: { values: [{ value: 'Application error' }] } }
|
||||
|
||||
expect(filterSentryEvent(event, enabled) === event).toBe(isSent)
|
||||
})
|
||||
|
||||
it.each([undefined, false, 'false', null, 1])(
|
||||
'drops errors from outside the app unless marked as a page crash: %s',
|
||||
(tag) => {
|
||||
|
||||
@@ -2,9 +2,12 @@ type SentryEventTags = {
|
||||
tags?: {
|
||||
globalErrorBoundary?: string | number | boolean | null
|
||||
third_party_code?: string | number | boolean | null
|
||||
codeSampleRate?: string | number | boolean | null
|
||||
}
|
||||
}
|
||||
|
||||
const NON_CRASH_ERROR_SAMPLE_RATE = 0.01
|
||||
|
||||
export function isSentryErrorBoundaryCrash(event: SentryEventTags): boolean {
|
||||
return event.tags?.globalErrorBoundary === true || event.tags?.globalErrorBoundary === 'true'
|
||||
}
|
||||
@@ -15,8 +18,17 @@ export function filterSentryEvent<T extends SentryEventTags>(
|
||||
): T | null {
|
||||
if (!isPlatform || !hasConsent) return null
|
||||
|
||||
const isErrorBoundaryCrash = isSentryErrorBoundaryCrash(event)
|
||||
const isThirdPartyOnly =
|
||||
event.tags?.third_party_code === true || event.tags?.third_party_code === 'true'
|
||||
|
||||
return isThirdPartyOnly && !isSentryErrorBoundaryCrash(event) ? null : event
|
||||
if (isThirdPartyOnly && !isErrorBoundaryCrash) return null
|
||||
if (!isErrorBoundaryCrash && Math.random() >= NON_CRASH_ERROR_SAMPLE_RATE) return null
|
||||
|
||||
event.tags = {
|
||||
...event.tags,
|
||||
codeSampleRate: isErrorBoundaryCrash ? '1' : NON_CRASH_ERROR_SAMPLE_RATE.toString(),
|
||||
}
|
||||
|
||||
return event
|
||||
}
|
||||
Reference in new issue
Block a user