Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,9 @@ export class EventsController {

return { message: 'Events emitted' };
}

@Get('test-isolation')
testIsolation() {
return { message: 'ok' };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@ import { EventEmitter2 } from '@nestjs/event-emitter';

@Injectable()
export class EventsService {
constructor(private readonly eventEmitter: EventEmitter2) {}
constructor(private readonly eventEmitter: EventEmitter2) {
// Emit event periodically outside of HTTP context to test isolation scope behavior.
// setInterval runs in the default async context (no HTTP request), so without proper
// isolation scope forking, the breadcrumb set by the handler leaks into the default
// isolation scope and gets cloned into subsequent HTTP requests.
setInterval(() => {
this.eventEmitter.emit('test-isolation.breadcrumb');
}, 2000);
}

async emitEvents() {
await this.eventEmitter.emit('myEvent.pass', { data: 'test' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ export class TestEventListener {
throw new Error('Test error from event handler');
}

@OnEvent('test-isolation.breadcrumb')
handleIsolationBreadcrumbEvent(): void {
Sentry.addBreadcrumb({
message: 'leaked-breadcrumb-from-event-handler',
level: 'info',
});
}

@OnEvent('multiple.first')
@OnEvent('multiple.second')
async handleMultipleEvents(payload: any): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,28 @@ test('Event emitter', async () => {
});
});

test('Event handler breadcrumbs do not leak into subsequent HTTP requests', async () => {
// The app emits 'test-isolation.breadcrumb' every 2s via setInterval (outside HTTP context).
// The handler adds a breadcrumb. Without isolation scope forking, this breadcrumb leaks
// into the default isolation scope and gets cloned into subsequent HTTP requests.

// Wait for at least one setInterval tick to fire and add the breadcrumb
await new Promise(resolve => setTimeout(resolve, 3000));

const transactionPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => {
return transactionEvent.transaction === 'GET /events/test-isolation';
});

await fetch('http://localhost:3050/events/test-isolation');

const transaction = await transactionPromise;

const leakedBreadcrumb = (transaction.breadcrumbs || []).find(
(b: any) => b.message === 'leaked-breadcrumb-from-event-handler',
);
expect(leakedBreadcrumb).toBeUndefined();
});

test('Multiple OnEvent decorators', async () => {
const firstTxPromise = waitForTransaction('nestjs-distributed-tracing', transactionEvent => {
return transactionEvent.transaction === 'event multiple.first|multiple.second';
Expand All @@ -64,6 +86,10 @@ test('Multiple OnEvent decorators', async () => {

expect(firstTx).toBeDefined();
expect(secondTx).toBeDefined();
// assert that the correct payloads were added
expect(rootTx.tags).toMatchObject({ 'test-first': true, 'test-second': true });

// Tags should be on the event handler transactions, not the root HTTP transaction
expect(firstTx.tags?.['test-first'] || firstTx.tags?.['test-second']).toBe(true);
expect(secondTx.tags?.['test-first'] || secondTx.tags?.['test-second']).toBe(true);
expect(rootTx.tags?.['test-first']).toBeUndefined();
expect(rootTx.tags?.['test-second']).toBeUndefined();
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
InstrumentationNodeModuleFile,
isWrapped,
} from '@opentelemetry/instrumentation';
import { captureException, SDK_VERSION, startSpan } from '@sentry/core';
import { captureException, SDK_VERSION, startSpan, withIsolationScope } from '@sentry/core';
import { getEventSpanOptions } from './helpers';
import type { OnEventTarget } from './types';

Expand Down Expand Up @@ -110,21 +110,23 @@ export class SentryNestEventInstrumentation extends InstrumentationBase {
}
}

return startSpan(getEventSpanOptions(eventName), async () => {
try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const result = await originalHandler.apply(this, args);
return result;
} catch (error) {
// exceptions from event handlers are not caught by global error filter
captureException(error, {
mechanism: {
handled: false,
type: 'auto.event.nestjs',
},
});
throw error;
}
return withIsolationScope(() => {
return startSpan(getEventSpanOptions(eventName), async () => {
try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const result = await originalHandler.apply(this, args);
return result;
} catch (error) {
// exceptions from event handlers are not caught by global error filter
captureException(error, {
mechanism: {
handled: false,
type: 'auto.event.nestjs',
},
});
throw error;
}
});
});
};

Expand Down
6 changes: 6 additions & 0 deletions packages/nestjs/test/integrations/nest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('Nest', () => {
} as OnEventTarget;
vi.spyOn(core, 'startSpan');
vi.spyOn(core, 'captureException');
vi.spyOn(core, 'withIsolationScope');
});

afterEach(() => {
Expand Down Expand Up @@ -75,6 +76,7 @@ describe('Nest', () => {

await descriptor.value();

expect(core.withIsolationScope).toHaveBeenCalled();
expect(core.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'event test.event',
Expand All @@ -90,6 +92,7 @@ describe('Nest', () => {

await descriptor.value();

expect(core.withIsolationScope).toHaveBeenCalled();
expect(core.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'event Symbol(test.event)',
Expand All @@ -105,6 +108,7 @@ describe('Nest', () => {

await descriptor.value();

expect(core.withIsolationScope).toHaveBeenCalled();
expect(core.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'event test.event1,test.event2',
Expand All @@ -120,6 +124,7 @@ describe('Nest', () => {

await descriptor.value();

expect(core.withIsolationScope).toHaveBeenCalled();
expect(core.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'event Symbol(test.event1),Symbol(test.event2)',
Expand All @@ -135,6 +140,7 @@ describe('Nest', () => {

await descriptor.value();

expect(core.withIsolationScope).toHaveBeenCalled();
expect(core.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'event Symbol(test.event1),test.event2,Symbol(test.event3)',
Expand Down
Loading