Skip to content

Commit c1991d6

Browse files
authored
fix(node): Ensure tool errors for vercelAiIntegration have correct trace connected (#17132)
This fixes the problem that for tool errors triggered from vercel-ai integration, traces were not connected. This seemed to happen because errors bubble up to the global unhandled rejection handler, where the span is no longer active and thus the trace cannot be connected. This PR fixes this by attaching the active span to the error and using this in the unhandled rejection handler, if it exists. Closes #17108
1 parent bc87e33 commit c1991d6

File tree

15 files changed

+564
-21
lines changed

15 files changed

+564
-21
lines changed

dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{
2-
"name": "node-express-app",
2+
"name": "aws-lambda-layer-cjs",
33
"version": "1.0.0",
44
"private": true,
5+
"type": "commonjs",
56
"scripts": {
67
"start": "node src/run.js",
78
"test": "playwright test",

dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Sentry.init({
55
dsn: 'https://public@dsn.ingest.sentry.io/1337',
66
release: '1.0',
77
transport: loggingTransport,
8-
tracesSampleRate: 1,
8+
tracesSampleRate: 0,
99
});
1010

1111
import express from 'express';

dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ afterAll(() => {
77

88
test('should capture and send Express controller error with txn name if tracesSampleRate is 0', async () => {
99
const runner = createRunner(__dirname, 'server.ts')
10-
.ignore('transaction')
1110
.expect({
1211
event: {
1312
exception: {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1,
8+
transport: loggingTransport,
9+
});
10+
11+
import express from 'express';
12+
13+
const app = express();
14+
15+
app.get('/test/express/:id', req => {
16+
throw new Error(`test_error with id ${req.params.id}`);
17+
});
18+
19+
Sentry.setupExpressErrorHandler(app);
20+
21+
startExpressServerAndSendPortToRunner(app);
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { afterAll, expect, test } from 'vitest';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
test('should capture and send Express controller error with txn name if tracesSampleRate is 1', async () => {
9+
const runner = createRunner(__dirname, 'server.ts')
10+
.expect({
11+
transaction: {
12+
transaction: 'GET /test/express/:id',
13+
},
14+
})
15+
.expect({
16+
event: {
17+
exception: {
18+
values: [
19+
{
20+
mechanism: {
21+
type: 'middleware',
22+
handled: false,
23+
},
24+
type: 'Error',
25+
value: 'test_error with id 123',
26+
stacktrace: {
27+
frames: expect.arrayContaining([
28+
expect.objectContaining({
29+
function: expect.any(String),
30+
lineno: expect.any(Number),
31+
colno: expect.any(Number),
32+
}),
33+
]),
34+
},
35+
},
36+
],
37+
},
38+
transaction: 'GET /test/express/:id',
39+
},
40+
})
41+
.start();
42+
runner.makeRequest('get', '/test/express/123', { expectError: true });
43+
await runner.completed();
44+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1,
8+
transport: loggingTransport,
9+
});
10+
11+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
12+
recordSpan(async () => {
13+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
14+
doSomething();
15+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
16+
doSomethingWithError();
17+
});
18+
19+
async function doSomething(): Promise<void> {
20+
return Promise.resolve();
21+
}
22+
23+
async function doSomethingWithError(): Promise<void> {
24+
await new Promise(resolve => setTimeout(resolve, 100));
25+
throw new Error('test error');
26+
}
27+
28+
// Duplicating some code from vercel-ai to verify how things work in more complex/weird scenarios
29+
function recordSpan(fn: (span: unknown) => Promise<void>): Promise<void> {
30+
return Sentry.startSpanManual({ name: 'test-span' }, async span => {
31+
try {
32+
const result = await fn(span);
33+
span.end();
34+
return result;
35+
} catch (error) {
36+
try {
37+
span.setStatus({ code: 2 });
38+
} finally {
39+
// always stop the span when there is an error:
40+
span.end();
41+
}
42+
43+
throw error;
44+
}
45+
});
46+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1,
8+
transport: loggingTransport,
9+
});
10+
11+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
12+
Sentry.startSpan({ name: 'test-span' }, async () => {
13+
throw new Error('test error');
14+
});

dev-packages/node-integration-tests/suites/public-api/onUnhandledRejectionIntegration/test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { Event } from '@sentry/node';
12
import * as childProcess from 'child_process';
23
import * as path from 'path';
34
import { afterAll, describe, expect, test } from 'vitest';
@@ -123,4 +124,58 @@ test rejection`);
123124
.start()
124125
.completed();
125126
});
127+
128+
test('handles unhandled rejection in spans', async () => {
129+
let transactionEvent: Event | undefined;
130+
let errorEvent: Event | undefined;
131+
132+
await createRunner(__dirname, 'scenario-with-span.ts')
133+
.expect({
134+
transaction: transaction => {
135+
transactionEvent = transaction;
136+
},
137+
})
138+
.expect({
139+
event: event => {
140+
errorEvent = event;
141+
},
142+
})
143+
.start()
144+
.completed();
145+
146+
expect(transactionEvent).toBeDefined();
147+
expect(errorEvent).toBeDefined();
148+
149+
expect(transactionEvent!.transaction).toBe('test-span');
150+
151+
expect(transactionEvent!.contexts!.trace!.trace_id).toBe(errorEvent!.contexts!.trace!.trace_id);
152+
expect(transactionEvent!.contexts!.trace!.span_id).toBe(errorEvent!.contexts!.trace!.span_id);
153+
});
154+
155+
test('handles unhandled rejection in spans that are ended early', async () => {
156+
let transactionEvent: Event | undefined;
157+
let errorEvent: Event | undefined;
158+
159+
await createRunner(__dirname, 'scenario-with-span-ended.ts')
160+
.expect({
161+
transaction: transaction => {
162+
transactionEvent = transaction;
163+
},
164+
})
165+
.expect({
166+
event: event => {
167+
errorEvent = event;
168+
},
169+
})
170+
.start()
171+
.completed();
172+
173+
expect(transactionEvent).toBeDefined();
174+
expect(errorEvent).toBeDefined();
175+
176+
expect(transactionEvent!.transaction).toBe('test-span');
177+
178+
expect(transactionEvent!.contexts!.trace!.trace_id).toBe(errorEvent!.contexts!.trace!.trace_id);
179+
expect(transactionEvent!.contexts!.trace!.span_id).toBe(errorEvent!.contexts!.trace!.span_id);
180+
});
126181
});

dev-packages/node-integration-tests/suites/tracing/vercelai/instrument-with-pii.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ Sentry.init({
77
tracesSampleRate: 1.0,
88
sendDefaultPii: true,
99
transport: loggingTransport,
10-
integrations: [Sentry.vercelAIIntegration({ force: true })],
10+
integrations: [Sentry.vercelAIIntegration()],
1111
});

dev-packages/node-integration-tests/suites/tracing/vercelai/instrument.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ Sentry.init({
66
release: '1.0',
77
tracesSampleRate: 1.0,
88
transport: loggingTransport,
9-
integrations: [Sentry.vercelAIIntegration({ force: true })],
9+
integrations: [Sentry.vercelAIIntegration()],
1010
});

0 commit comments

Comments
 (0)