Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(core): Isolate throwing user callbacks instead of capturing them as events#23770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| transport: loggingTransport, | ||
| beforeSend() { | ||
| throw new Error('beforeSend failed'); | ||
| }, | ||
| }); | ||
| Sentry.captureException(new Error('this should get dropped because beforeSend throws')); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| Sentry.flush(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { afterAll, test } from 'vitest'; | ||
| import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
| test('records a client report and no extra error event when beforeSend throws', async () => { | ||
| await createRunner(__dirname, 'scenario.ts') | ||
| .unignore('client_report') | ||
| .expect({ | ||
| client_report: { | ||
| discarded_events: [ | ||
| { | ||
| category: 'error', | ||
| quantity: 1, | ||
| reason: 'before_send', | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| transport: loggingTransport, | ||
| }); | ||
| Sentry.addEventProcessor(async () => { | ||
| throw new Error('async event processor failed'); | ||
| }); | ||
| Sentry.captureException(new Error('this should get dropped because the async event processor rejects')); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| Sentry.flush(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| transport: loggingTransport, | ||
| }); | ||
| Sentry.addEventProcessor(() => { | ||
| throw new Error('event processor failed'); | ||
| }); | ||
| Sentry.captureException(new Error('this should get dropped because the event processor throws')); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| Sentry.flush(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { afterAll, test } from 'vitest'; | ||
| import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
| test('records a client report and no extra error event when an event processor throws', async () => { | ||
| await createRunner(__dirname, 'scenario.ts') | ||
| .unignore('client_report') | ||
| .expect({ | ||
| client_report: { | ||
| discarded_events: [ | ||
| { | ||
| category: 'error', | ||
| quantity: 1, | ||
| reason: 'event_processor', | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); | ||
| test('records a client report and no extra error event when an async event processor rejects', async () => { | ||
| await createRunner(__dirname, 'scenario-async.ts') | ||
| .unignore('client_report') | ||
| .expect({ | ||
| client_report: { | ||
| discarded_events: [ | ||
| { | ||
| category: 'error', | ||
| quantity: 1, | ||
| reason: 'event_processor', | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| transport: loggingTransport, | ||
| tracesSampleRate: 1, | ||
| tracesSampler: () => { | ||
| throw new Error('tracesSampler failed'); | ||
| }, | ||
| }); | ||
| Sentry.startSpan({ name: 'sampled via tracesSampleRate fallback' }, () => { | ||
| // no-op | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| Sentry.flush(); |
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: can we add a test what happens when we set |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| transport: loggingTransport, | ||
| tracesSampler: () => { | ||
| throw new Error('tracesSampler failed'); | ||
| }, | ||
| }); | ||
| Sentry.startSpan({ name: 'this should not be sampled because tracesSampler throws' }, () => { | ||
| // no-op | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| Sentry.flush(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { afterAll, test } from 'vitest'; | ||
| import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
| test('records a client report and no error event when tracesSampler throws', async () => { | ||
| await createRunner(__dirname, 'scenario.ts') | ||
| .unignore('client_report') | ||
| .expect({ | ||
| client_report: { | ||
| discarded_events: [ | ||
| { | ||
| category: 'span', | ||
| quantity: 1, | ||
| reason: 'sample_rate', | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); | ||
| test('sends the span when tracesSampler throws but tracesSampleRate is 1', async () => { | ||
| await createRunner(__dirname, 'scenario-fallback.ts') | ||
| .expect({ | ||
| transaction: { | ||
| transaction: 'sampled via tracesSampleRate fallback', | ||
| }, | ||
| }) | ||
| .start() | ||
| .completed(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -3,6 +3,7 @@ import type { Event, EventHint } from './types/event'; | ||
| import type { EventProcessor } from './types/eventprocessor'; | ||
| import { debug } from './utils/debug-logger'; | ||
| import { isThenable } from './utils/is'; | ||
| import { safeCallback } from './utils/safeCallback'; | ||
| import { rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise'; | ||
| /** | ||
| @@ -34,9 +35,15 @@ function _notifyEventProcessors( | ||
| return event; | ||
| } | ||
| const result = processor({ ...event }, hint); | ||
| const processorName = `Event processor "${processor.id || '?'}"`; | ||
| DEBUG_BUILD && result === null && debug.log(`Event processor "${processor.id || '?'}" dropped event`); | ||
| const result = safeCallback( | ||
| DEBUG_BUILD ? `${processorName} threw an error, dropping event:` : '', | ||
| () => processor({ ...event }, hint), | ||
| () => null, | ||
| ); | ||
| DEBUG_BUILD && result === null && debug.log(`${processorName} dropped event`); | ||
sentry[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (isThenable(result)) { | ||
| return result.then(final => _notifyEventProcessors(final, hint, processors, index + 1)); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could also have a second scenario where we throw in an async event processor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes good idea, added 👍