Skip to content
Open
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
@@ -1,7 +1,6 @@
import * as Sentry from '@sentry/nestjs';

Sentry.init({
traceLifecycle: 'static',
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: process.env.E2E_TEST_DSN,
tunnel: `http://localhost:3031/`, // proxy server
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { expect, test } from '@playwright/test';
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
import { collectStreamedSpansUntilSegment, waitForError } from '@sentry-internal/test-utils';

const APP_NAME = 'nestjs-12';

test('Sends exception to Sentry', async ({ baseURL }) => {
const errorEventPromise = waitForError('nestjs-12', event => {
const errorEventPromise = waitForError(APP_NAME, event => {
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
});

Expand Down Expand Up @@ -37,38 +39,35 @@ test('Sends exception to Sentry', async ({ baseURL }) => {
test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
let errorEventOccurred = false;

waitForError('nestjs-12', event => {
waitForError(APP_NAME, event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 400 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-400-exception/:id';
});

waitForError('nestjs-12', event => {
waitForError(APP_NAME, event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected 500 exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-500-exception/:id';
});

const transactionEventPromise400 = waitForTransaction('nestjs-12', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise500 = waitForTransaction('nestjs-12', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id';
});
// Waiting for each request's segment span is how this spec knows the request finished and
// any error it would have produced had its chance to be sent.
const spansPromise400 = collectStreamedSpansUntilSegment(APP_NAME, 'GET /test-expected-400-exception/:id');
const spansPromise500 = collectStreamedSpansUntilSegment(APP_NAME, 'GET /test-expected-500-exception/:id');

const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`);
expect(response400.status).toBe(400);

const response500 = await fetch(`${baseURL}/test-expected-500-exception/123`);
expect(response500.status).toBe(500);

await transactionEventPromise400;
await transactionEventPromise500;
await spansPromise400;
await spansPromise500;

(await fetch(`${baseURL}/flush`)).text();

Expand All @@ -78,22 +77,20 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => {
let errorEventOccurred = false;

waitForError('nestjs-12', event => {
waitForError(APP_NAME, event => {
if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected RPC exception with id 123') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /test-expected-rpc-exception/:id';
});

const transactionEventPromise = waitForTransaction('nestjs-12', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id';
});
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /test-expected-rpc-exception/:id');

const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`);
expect(response.status).toBe(500);

await transactionEventPromise;
await spansPromise;

(await fetch(`${baseURL}/flush`)).text();

Expand All @@ -105,17 +102,15 @@ test('Global exception filter registered in main module is applied and exception
}) => {
let errorEventOccurred = false;

waitForError('nestjs-12', event => {
waitForError(APP_NAME, event => {
if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by global filter!') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /example-exception-global-filter';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: waitForError calls rely on transaction events that are no longer sent with span streaming, creating unresolved promises in the background during tests.
Severity: LOW

Suggested Fix

Remove the waitForError calls that rely on transaction events. The test synchronization has been updated to use collectStreamedSpansUntilSegment, so the old waitForError calls are now obsolete and incorrect.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: dev-packages/e2e-tests/test-applications/nestjs-12/tests/errors.test.ts#L110

Potential issue: In `errors.test.ts`, the `waitForError` helper is used with a callback
that resolves a promise by matching the `transaction` property on an event. However,
with span streaming enabled, transaction-type events are no longer sent to the test
proxy. As a result, the callback condition is never met, and the promise returned by
`waitForError` never resolves. These unawaited, unresolved promises accumulate in the
background, creating a logical flaw in the test's synchronization logic.

Also affects:

  • dev-packages/e2e-tests/test-applications/nestjs-12/tests/errors.test.ts:143~143

Did we get this right? 👍 / 👎 to inform future reviews.

});

const transactionEventPromise = waitForTransaction('nestjs-12', transactionEvent => {
return transactionEvent?.transaction === 'GET /example-exception-global-filter';
});
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /example-exception-global-filter');

const response = await fetch(`${baseURL}/example-exception-global-filter`);
const responseBody = await response.json();
Expand All @@ -128,7 +123,7 @@ test('Global exception filter registered in main module is applied and exception
message: 'Example exception was handled by global filter!',
});

await transactionEventPromise;
await spansPromise;

(await fetch(`${baseURL}/flush`)).text();

Expand All @@ -140,17 +135,15 @@ test('Local exception filter registered in main module is applied and exception
}) => {
let errorEventOccurred = false;

waitForError('nestjs-12', event => {
waitForError(APP_NAME, event => {
if (!event.type && event.exception?.values?.[0]?.value === 'Example exception was handled by local filter!') {
errorEventOccurred = true;
}

return event?.transaction === 'GET /example-exception-local-filter';
});

const transactionEventPromise = waitForTransaction('nestjs-12', transactionEvent => {
return transactionEvent?.transaction === 'GET /example-exception-local-filter';
});
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /example-exception-local-filter');

const response = await fetch(`${baseURL}/example-exception-local-filter`);
const responseBody = await response.json();
Expand All @@ -163,7 +156,7 @@ test('Local exception filter registered in main module is applied and exception
message: 'Example exception was handled by local filter!',
});

await transactionEventPromise;
await spansPromise;

(await fetch(`${baseURL}/flush`)).text();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,73 +1,51 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';
import { collectStreamedSpansUntilSegment } from '@sentry-internal/test-utils';

test('Transaction includes span and correct value for decorated async function', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs-12', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-span-decorator-async'
);
});
const APP_NAME = 'nestjs-12';

test('Trace includes span and correct value for decorated async function', async ({ baseURL }) => {
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /test-span-decorator-async');

const response = await fetch(`${baseURL}/test-span-decorator-async`);
const body = await response.json();

expect(body.result).toEqual('test');

const transactionEvent = await transactionEventPromise;
const spans = await spansPromise;

expect(transactionEvent.spans).toEqual(
expect.arrayContaining([
expect.objectContaining({
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'sentry.origin': 'auto.function.nestjs.sentry_traced',
'sentry.op': 'wait and return a string',
},
description: 'wait',
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
start_timestamp: expect.any(Number),
status: 'ok',
op: 'wait and return a string',
origin: 'auto.function.nestjs.sentry_traced',
expect(spans).toContainEqual(
expect.objectContaining({
name: 'wait',
is_segment: false,
status: 'ok',
attributes: expect.objectContaining({
'sentry.origin': { type: 'string', value: 'auto.function.nestjs.sentry_traced' },
'sentry.op': { type: 'string', value: 'wait and return a string' },
}),
]),
}),
);
});

test('Transaction includes span and correct value for decorated sync function', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('nestjs-12', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-span-decorator-sync'
);
});
test('Trace includes span and correct value for decorated sync function', async ({ baseURL }) => {
const spansPromise = collectStreamedSpansUntilSegment(APP_NAME, 'GET /test-span-decorator-sync');

const response = await fetch(`${baseURL}/test-span-decorator-sync`);
const body = await response.json();

expect(body.result).toEqual('test');

const transactionEvent = await transactionEventPromise;
const spans = await spansPromise;

expect(transactionEvent.spans).toEqual(
expect.arrayContaining([
expect.objectContaining({
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'sentry.origin': 'auto.function.nestjs.sentry_traced',
'sentry.op': 'return a string',
},
description: 'getString',
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
start_timestamp: expect.any(Number),
status: 'ok',
op: 'return a string',
origin: 'auto.function.nestjs.sentry_traced',
expect(spans).toContainEqual(
expect.objectContaining({
name: 'getString',
is_segment: false,
status: 'ok',
attributes: expect.objectContaining({
'sentry.origin': { type: 'string', value: 'auto.function.nestjs.sentry_traced' },
'sentry.op': { type: 'string', value: 'return a string' },
}),
]),
}),
);
});

Expand Down
Loading
Loading