Skip to content

Commit

Permalink
test: Add tests to demonstrate withScope data loss for uncaught err…
Browse files Browse the repository at this point in the history
…ors (getsentry#11204)

This PR adds integration tests (browser and node) to demonstrate the
current behavior around loosing scope data when a thrown error is caught
by global handlers "outside" the lifetime of the throw origin's scope.
This occurs specifically within the `withScope` API where the scope is
popped if an error is caught but then the error is rethrown to be caught
by handlers down the line.
  • Loading branch information
Lms24 authored and cadesalaberry committed Apr 19, 2024
1 parent 6dd8539 commit 0791e50
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Sentry.setTag('global', 'tag');
setTimeout(() => {
Sentry.withScope(scope => {
scope.setTag('local', 'tag');
throw new Error('test error');
});
}, 10);
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';

/**
* Why does this test exist?
*
* We recently discovered that errors caught by global handlers will potentially loose scope data from the active scope
* where the error was thrown in. The simple example in this test (see subject.ts) demonstrates this behavior (in a
* browser environment but the same behavior applies to the server; see the test there).
*
* This test nevertheless covers the behavior so that we're aware.
*/
sentryTest(
'withScope scope is NOT applied to thrown error caught by global handler',
async ({ getLocalTestPath, page }) => {
const url = await getLocalTestPath({ testDir: __dirname });

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);

const ex = eventData.exception?.values ? eventData.exception.values[0] : undefined;

// This tag is missing :(
expect(eventData.tags?.local).toBeUndefined();

expect(eventData.tags).toMatchObject({
global: 'tag',
});
expect(ex?.value).toBe('test error');
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node-experimental';
import express from 'express';

const app = express();

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
transport: loggingTransport,
});

app.use(Sentry.Handlers.requestHandler());

Sentry.setTag('global', 'tag');

app.get('/test/withScope', () => {
Sentry.withScope(scope => {
scope.setTag('local', 'tag');
throw new Error('test_error');
});
});

app.get('/test/isolationScope', () => {
Sentry.getIsolationScope().setTag('isolation-scope', 'tag');
throw new Error('isolation_test_error');
});

app.use(Sentry.Handlers.errorHandler());

startExpressServerAndSendPortToRunner(app);
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

/**
* Why does this test exist?
*
* We recently discovered that errors caught by global handlers will potentially loose scope data from the active scope
* where the error was originally thrown in. The simple example in this test (see subject.ts) demonstrates this behavior
* (in a Node environment but the same behavior applies to the browser; see the test there).
*
* This test nevertheless covers the behavior so that we're aware.
*/
test('withScope scope is NOT applied to thrown error caught by global handler', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
exception: {
values: [
{
mechanism: {
type: 'middleware',
handled: false,
},
type: 'Error',
value: 'test_error',
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
function: expect.any(String),
lineno: expect.any(Number),
colno: expect.any(Number),
}),
]),
},
},
],
},
// 'local' tag is not applied to the event
tags: expect.not.objectContaining({ local: expect.anything() }),
},
})
.start(done);

expect(() => runner.makeRequest('get', '/test/withScope')).rejects.toThrow();
});

/**
* This test shows that the isolation scope set tags are applied correctly to the error.
*/
test('isolation scope is applied to thrown error caught by global handler', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
exception: {
values: [
{
mechanism: {
type: 'middleware',
handled: false,
},
type: 'Error',
value: 'isolation_test_error',
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
function: expect.any(String),
lineno: expect.any(Number),
colno: expect.any(Number),
}),
]),
},
},
],
},
tags: {
global: 'tag',
'isolation-scope': 'tag',
},
},
})
.start(done);

expect(() => runner.makeRequest('get', '/test/isolationScope')).rejects.toThrow();
});

0 comments on commit 0791e50

Please sign in to comment.