fix(node): Ensure tool errors for vercelAiIntegration have correct trace connected
#17132
Conversation
size-limit report 📦
|
| @@ -7,5 +7,5 @@ Sentry.init({ | |||
| tracesSampleRate: 1.0, | |||
| sendDefaultPii: true, | |||
| transport: loggingTransport, | |||
| integrations: [Sentry.vercelAIIntegration({ force: true })], | |||
There was a problem hiding this comment.
leftover that should not be needed anymore
| // So to circumvent this, we set the active span on the error object | ||
| // which is picked up by the unhandledrejection handler | ||
| if (error && typeof error === 'object') { | ||
| addNonEnumerableProperty(error, '_sentry_active_span', getActiveSpan()); |
There was a problem hiding this comment.
fwiw addNonEnumerableProperty is really slow, but I guess in error codepaths this is fine.
| cleanupChildProcesses(); | ||
| }); | ||
|
|
||
| test('should capture and send Express controller error with txn name if tracesSampleRate is 1', async () => { |
There was a problem hiding this comment.
noticed this test was missing, just adding for completeness sake.
| @@ -5,7 +5,7 @@ Sentry.init({ | |||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | |||
| release: '1.0', | |||
| transport: loggingTransport, | |||
| tracesSampleRate: 1, | |||
| tracesSampleRate: 0, | |||
There was a problem hiding this comment.
this was actually not testing what the test said it would ^^
| @@ -0,0 +1,50 @@ | |||
| import * as Sentry from '@sentry/node'; | |||
| import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | |||
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
mydea
force-pushed
the
fn/vercel-ai-tests
branch
from
3da5545 to
08659ea
Compare
There was a problem hiding this comment.
Bug: Transaction Sampling Test Fails
The test removes .ignore('transaction'), causing it to expect transaction events. This conflicts with the server's tracesSampleRate: 0 configuration, which prevents transaction sampling. Consequently, the test fails because no transaction event is generated, despite the test's intent to verify behavior when tracesSampleRate is 0.
dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/test.ts#L8-L14
mydea
force-pushed
the
fn/vercel-ai-tests
branch
from
08659ea to
841cfef
Compare
mydea
force-pushed
the
fn/vercel-ai-tests
branch
from
841cfef to
59e546e
Compare
mydea
changed the title
vercelAiIntegration have correct trace connected
There was a problem hiding this comment.
Bug: Test Hangs Due to Zero Traces Sample Rate
The test removed .ignore('transaction') but the server is configured with tracesSampleRate: 0. Since tracesSampleRate: 0 prevents transaction events from being sent, the test runner will hang or timeout waiting for them. The test's purpose is to verify that error events still include transaction names even when transactions are not sampled, so it should continue to ignore transaction events.
dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/test.ts#L8-L14
…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
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