From 4c52da887a9237484616eafdfcb3bb10bb6cfbd9 Mon Sep 17 00:00:00 2001 From: Abhilaksh Sharma Date: Tue, 4 Aug 2026 19:48:19 +0530 Subject: [PATCH] fix(server-utils): Don't capture AI client errors as unhandled at the instrumentation level The exported instrumentOpenAiClient, instrumentAnthropicAiClient and instrumentGoogleGenAIClient wrappers captured provider errors with mechanism.handled = false and then rethrew, so the SDK classified the error as an unhandled crash before the application's retry or fallback logic ran. A call that succeeded on retry still produced an unhandled event, and each retry produced another one. Applies the convention established for the channel-based OpenAI integration in #21877 to the manual client instrumentation, which is the only available path on the edge and serverless runtimes. Error span status and the original error identity are unchanged. The anthropic suite asserted this divergence directly, expecting the model-error event only while orchestrion was disabled. Both paths agree now, so that branch and its expectation are gone. Captures are kept where a provider reports an error as data on an otherwise successful call, since the caller never sees those as a thrown error. The AI integration suites no longer mask these events with .ignore('event'), so they fail if the capture returns. Dropping the capture also left the .catch() in createWithResponseWrapper rethrowing into a promise nothing observes, which is not a handler at all: awaiting the two promises in sequence orphans the second whenever the first rejects. They are awaited together now. Co-Authored-By: Claude Opus 5 --- .../suites/tracing/anthropic/test.ts | 53 +------ .../suites/tracing/google-genai/test.ts | 10 +- .../tracing/openai/openai-tool-calls/test.ts | 2 - .../suites/tracing/openai/test.ts | 12 -- .../suites/tracing/openai/v6/test.ts | 5 - .../server-utils/src/ai/anthropic-ai/index.ts | 44 ++---- packages/server-utils/src/ai/core/utils.ts | 24 +--- .../server-utils/src/ai/google-genai/index.ts | 16 +-- packages/server-utils/src/ai/openai/index.ts | 32 +---- .../test/ai/lib/tracing/ai/utils.test.ts | 10 +- .../tracing/ai-client-error-capture.test.ts | 132 ++++++++++++++++++ 11 files changed, 169 insertions(+), 171 deletions(-) create mode 100644 packages/server-utils/test/ai/tracing/ai-client-error-capture.test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/anthropic/test.ts b/dev-packages/node-integration-tests/suites/tracing/anthropic/test.ts index f1c94b2b1dd6..f1c8a8b054e5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/anthropic/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/anthropic/test.ts @@ -20,7 +20,6 @@ import { GEN_AI_USAGE_TOTAL_TOKENS, } from '@sentry/conventions/attributes'; import { GEN_AI_REQUEST_STREAM_ATTRIBUTE } from '../../../../../packages/server-utils/src/ai/core/gen-ai-attributes'; -import { isOrchestrionEnabled } from '../../../utils'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; describe('Anthropic integration', () => { @@ -38,17 +37,6 @@ describe('Anthropic integration', () => { expect(modelsSpan!.status).toBe('ok'); }; - const EXPECTED_MODEL_ERROR = { - exception: { - values: [ - { - type: 'Error', - value: '404 Model not found', - }, - ], - }, - }; - const EXPECTED_STREAM_EVENT_HANDLER_MESSAGE = { message: 'stream event from user-added event listener captured', }; @@ -56,7 +44,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-with-response.mjs', 'instrument.mjs', (createRunner, test) => { test('preserves .withResponse() and .asResponse() for non-streaming and streaming', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -92,15 +79,7 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('creates anthropic related spans with genAI recording disabled', async () => { - const runner = createRunner(); - - // The orchestrion path only marks the errored span; unlike the OTel path it does not - // capture the handled `error-model` rejection as an event. - if (!isOrchestrionEnabled()) { - runner.expect({ event: EXPECTED_MODEL_ERROR }); - } - - await runner + await createRunner() .expect({ transaction: expectModelsSpanOnTransaction }) .expect({ span: container => { @@ -142,15 +121,7 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates anthropic related spans with genAI recording enabled', async () => { - const runner = createRunner(); - - // The orchestrion path only marks the errored span; unlike the OTel path it does not - // capture the handled `error-model` rejection as an event. - if (!isOrchestrionEnabled()) { - runner.expect({ event: EXPECTED_MODEL_ERROR }); - } - - await runner + await createRunner() .expect({ transaction: expectModelsSpanOnTransaction }) .expect({ span: container => { @@ -220,15 +191,7 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-with-options.mjs', (createRunner, test) => { test('creates anthropic related spans with custom options', async () => { - const runner = createRunner(); - - // The orchestrion path only marks the errored span; unlike the OTel path it does not - // capture the handled `error-model` rejection as an event. - if (!isOrchestrionEnabled()) { - runner.expect({ event: EXPECTED_MODEL_ERROR }); - } - - await runner + await createRunner() .expect({ transaction: event => { expect(event.transaction).toBe('main'); @@ -296,7 +259,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-stream.mjs', 'instrument.mjs', (createRunner, test) => { test('streams produce spans with token usage and metadata (PII false)', async () => { await createRunner() - .ignore('event') .expect({ transaction: EXPECTED_STREAM_SPANS_PII_FALSE }) .expect({ span: container => { @@ -350,7 +312,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-stream.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('streams record response text when PII true', async () => { await createRunner() - .ignore('event') .expect({ transaction: EXPECTED_STREAM_SPANS_PII_TRUE }) .expect({ span: container => { @@ -401,7 +362,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-stream-nested-create.mjs', 'instrument.mjs', (createRunner, test) => { test('traces a create() invoked from a stream event handler (dedup does not over-suppress)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -432,7 +392,6 @@ describe('Anthropic integration', () => { const EXPECTED_TOOL_CALLS_JSON = '[{"type":"tool_use","id":"tool_weather_1","name":"weather","input":{"city":"Paris"}}]'; await createRunner() - .ignore('event') .expect({ transaction: {}, }) @@ -462,7 +421,6 @@ describe('Anthropic integration', () => { const EXPECTED_TOOL_CALLS_JSON = '[{"type":"tool_use","id":"tool_weather_2","name":"weather","input":{"city":"Paris"}}]'; await createRunner() - .ignore('event') .expect({ transaction: {}, }) @@ -503,6 +461,8 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-stream-errors.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('handles streaming errors correctly', async () => { await createRunner() + // Anthropic surfaces stream errors as events on a resolved stream rather than by rejecting, + // so the instrumentation still reports them; the caller never sees them as a thrown error. .ignore('event') .expect({ transaction: EXPECTED_STREAM_ERROR_SPANS }) .expect({ @@ -551,7 +511,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario-errors.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('handles tool errors and model retrieval errors correctly', async () => { await createRunner() - .ignore('event') .expect({ transaction: event => { expect(event.transaction).toBe('main'); @@ -590,7 +549,6 @@ describe('Anthropic integration', () => { test('extracts system instructions from messages', async () => { const expectedInstructions = JSON.stringify([{ type: 'text', content: 'You are a helpful assistant' }]); await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -614,7 +572,6 @@ describe('Anthropic integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-span-streaming.mjs', (createRunner, test) => { test('creates anthropic related spans with span streaming enabled', async () => { await createRunner() - .ignore('event') .expect({ span: container => { const completionSpan = container.items.find( diff --git a/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts b/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts index de405368d40c..21364a25a149 100644 --- a/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/google-genai/test.ts @@ -32,7 +32,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('creates google genai related spans with genAI recording disabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -86,7 +85,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates google genai related spans with genAI recording enabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -137,7 +135,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-with-options.mjs', (createRunner, test) => { test('creates google genai related spans with custom options', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -174,7 +171,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-tools.mjs', 'instrument-with-options.mjs', (createRunner, test) => { test('creates google genai related spans with tool calls', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -233,6 +229,7 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-streaming.mjs', 'instrument.mjs', (createRunner, test) => { test('creates google genai streaming spans with genAI recording disabled', async () => { await createRunner() + // Blocked content is reported from within the stream, not thrown to the caller. .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ @@ -289,6 +286,7 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-streaming.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates google genai streaming spans with genAI recording enabled', async () => { await createRunner() + // Blocked content is reported from within the stream, not thrown to the caller. .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ @@ -348,7 +346,6 @@ describe('Google GenAI integration', () => { (createRunner, test) => { test('extracts system instructions from messages', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -372,7 +369,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-embeddings.mjs', 'instrument.mjs', (createRunner, test) => { test('creates google genai embeddings spans with genAI recording disabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -411,7 +407,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-embeddings.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates google genai embeddings spans with genAI recording enabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -458,7 +453,6 @@ describe('Google GenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-span-streaming.mjs', (createRunner, test) => { test('creates google genai related spans with span streaming enabled', async () => { await createRunner() - .ignore('event') .expect({ span: container => { const generateContentSpan = container.items.find(span => span.name === 'generate_content gemini-1.5-flash'); diff --git a/dev-packages/node-integration-tests/suites/tracing/openai/openai-tool-calls/test.ts b/dev-packages/node-integration-tests/suites/tracing/openai/openai-tool-calls/test.ts index b14d5a56901f..0f6be4aea592 100644 --- a/dev-packages/node-integration-tests/suites/tracing/openai/openai-tool-calls/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/openai/openai-tool-calls/test.ts @@ -78,7 +78,6 @@ describe('OpenAI Tool Calls integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('creates openai tool calls related spans with genAI recording disabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -326,7 +325,6 @@ describe('OpenAI Tool Calls integration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates openai tool calls related spans with genAI recording enabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { diff --git a/dev-packages/node-integration-tests/suites/tracing/openai/test.ts b/dev-packages/node-integration-tests/suites/tracing/openai/test.ts index 13c53a7d164f..4b166d7a38d2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/openai/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/openai/test.ts @@ -33,7 +33,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-chat.mjs', 'instrument.mjs', (createRunner, test) => { test('creates openai related spans with genAI recording disabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -333,7 +332,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-chat.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates openai related spans with genAI recording enabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -681,7 +679,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-chat.mjs', 'instrument-with-options.mjs', (createRunner, test) => { test('creates openai related spans with custom options', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -726,7 +723,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-embeddings.mjs', 'instrument.mjs', (createRunner, test) => { test('creates openai related spans with genAI recording disabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -859,7 +855,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-embeddings.mjs', 'instrument-with-pii.mjs', (createRunner, test) => { test('creates openai related spans with genAI recording enabled', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -1089,7 +1084,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-conversation.mjs', 'instrument.mjs', (createRunner, test) => { test('captures conversation ID from Conversations API and previous_response_id', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'conversation-test', @@ -1209,7 +1203,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-manual-conversation-id.mjs', 'instrument.mjs', (createRunner, test) => { test('attaches manual conversation ID set via setConversationId() to all chat spans', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'chat-with-manual-conversation-id', @@ -1242,7 +1235,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-separate-scope-1.mjs', 'instrument.mjs', (createRunner, test) => { test('isolates conversation IDs across separate scopes - conversation 1', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'GET /chat/conversation-1', @@ -1274,7 +1266,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-separate-scope-2.mjs', 'instrument.mjs', (createRunner, test) => { test('isolates conversation IDs across separate scopes - conversation 2', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'GET /chat/conversation-2', @@ -1310,7 +1301,6 @@ describe('OpenAI integration', () => { (createRunner, test) => { test('extracts system instructions from messages', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -1337,7 +1327,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-with-response.mjs', 'instrument.mjs', (createRunner, test) => { test('preserves .withResponse() method and works correctly', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -1368,7 +1357,6 @@ describe('OpenAI integration', () => { createEsmAndCjsTests(__dirname, 'scenario-chat.mjs', 'instrument-span-streaming.mjs', (createRunner, test) => { test('creates openai related spans with span streaming enabled', async () => { await createRunner() - .ignore('event') .expect({ span: container => { const chatCompletionSpan = container.items.find( diff --git a/dev-packages/node-integration-tests/suites/tracing/openai/v6/test.ts b/dev-packages/node-integration-tests/suites/tracing/openai/v6/test.ts index 3362a48159b8..091867486e4e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/openai/v6/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/openai/v6/test.ts @@ -36,7 +36,6 @@ describe('OpenAI integration (V6)', () => { (createRunner, test) => { test('creates openai related spans with genAI recording disabled (v6)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -346,7 +345,6 @@ describe('OpenAI integration (V6)', () => { (createRunner, test) => { test('creates openai related spans with genAI recording enabled (v6)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -704,7 +702,6 @@ describe('OpenAI integration (V6)', () => { (createRunner, test) => { test('creates openai related spans with custom options (v6)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main' } }) .expect({ span: container => { @@ -767,7 +764,6 @@ describe('OpenAI integration (V6)', () => { (createRunner, test) => { test('creates openai related spans with genAI recording disabled (v6)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', @@ -910,7 +906,6 @@ describe('OpenAI integration (V6)', () => { (createRunner, test) => { test('creates openai related spans with genAI recording enabled (v6)', async () => { await createRunner() - .ignore('event') .expect({ transaction: { transaction: 'main', diff --git a/packages/server-utils/src/ai/anthropic-ai/index.ts b/packages/server-utils/src/ai/anthropic-ai/index.ts index 1d0f007f18ed..6d1eef0daad2 100644 --- a/packages/server-utils/src/ai/anthropic-ai/index.ts +++ b/packages/server-utils/src/ai/anthropic-ai/index.ts @@ -1,11 +1,5 @@ /* eslint-disable typescript-eslint/no-deprecated */ -import { - captureException, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SPAN_STATUS_ERROR, - startSpan, - startSpanManual, -} from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startSpan, startSpanManual } from '@sentry/core'; import type { Span, SpanAttributeValue } from '@sentry/core'; import { GEN_AI_OPERATION_NAME, @@ -177,11 +171,7 @@ export function addResponseAttributes(span: Span, response: AnthropicAiResponse, /** * Handle common error catching and reporting for streaming requests */ -function handleStreamingError(error: unknown, span: Span, methodPath: string): never { - captureException(error, { - mechanism: { handled: false, type: 'auto.ai.anthropic', data: { function: methodPath } }, - }); - +function handleStreamingError(error: unknown, span: Span): never { if (span.isRecording()) { span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); span.end(); @@ -231,12 +221,12 @@ function handleStreamingRequest( options.recordOutputs ?? false, ) as unknown as R; } catch (error) { - return handleStreamingError(error, span, methodPath); + return handleStreamingError(error, span); } })(); }); - return wrapPromiseWithMethods(originalResult, instrumentedPromise, 'auto.ai.anthropic'); + return wrapPromiseWithMethods(originalResult, instrumentedPromise); } else { return startSpanManual(spanConfig, span => { try { @@ -251,7 +241,7 @@ function handleStreamingRequest( return instrumentMessageStream(messageStream, span, options.recordOutputs ?? false); } catch (error) { suppressDelegatedCreate = false; - return handleStreamingError(error, span, methodPath); + return handleStreamingError(error, span); } }); } @@ -323,28 +313,14 @@ function instrumentMethod( addPrivateRequestAttributes(span, params); } - return originalResult.then( - result => { - addResponseAttributes(span, result as AnthropicAiResponse, options.recordOutputs); - return result; - }, - error => { - captureException(error, { - mechanism: { - handled: false, - type: 'auto.ai.anthropic', - data: { - function: methodPath, - }, - }, - }); - throw error; - }, - ); + return originalResult.then(result => { + addResponseAttributes(span, result as AnthropicAiResponse, options.recordOutputs); + return result; + }); }, ); - return wrapPromiseWithMethods(originalResult, instrumentedPromise, 'auto.ai.anthropic'); + return wrapPromiseWithMethods(originalResult, instrumentedPromise); }, }); } diff --git a/packages/server-utils/src/ai/core/utils.ts b/packages/server-utils/src/ai/core/utils.ts index c770778a3369..0f45301a99b7 100644 --- a/packages/server-utils/src/ai/core/utils.ts +++ b/packages/server-utils/src/ai/core/utils.ts @@ -2,7 +2,7 @@ /** * Shared utils for AI integrations (OpenAI, Anthropic, Verce.AI, etc.) */ -import { captureException, getClient, isThenable } from '@sentry/core'; +import { getClient, isThenable } from '@sentry/core'; import type { Span } from '@sentry/core'; import { GEN_AI_RESPONSE_FINISH_REASONS, @@ -230,22 +230,11 @@ export function extractSystemInstructions(messages: unknown[] | unknown): { async function createWithResponseWrapper( originalWithResponse: Promise, instrumentedPromise: Promise, - mechanismType: string, ): Promise { - // Attach catch handler to originalWithResponse immediately to prevent unhandled rejection - // If instrumentedPromise rejects first, we still need this handled - const safeOriginalWithResponse = originalWithResponse.catch(error => { - captureException(error, { - mechanism: { - handled: false, - type: mechanismType, - }, - }); - throw error; - }); - - const instrumentedResult = await instrumentedPromise; - const originalWrapper = await safeOriginalWithResponse; + // Awaited together rather than in sequence so both promises get a handler attached synchronously. + // Awaiting them one after the other leaves the second unobserved when the first rejects, which + // surfaces as an unhandled rejection. + const [instrumentedResult, originalWrapper] = await Promise.all([instrumentedPromise, originalWithResponse]); // Combine instrumented result with original metadata if (originalWrapper && typeof originalWrapper === 'object' && 'data' in originalWrapper) { @@ -268,7 +257,6 @@ async function createWithResponseWrapper( export function wrapPromiseWithMethods( originalPromiseLike: Promise, instrumentedPromise: Promise, - mechanismType: string, ): Promise { // If the original result is not thenable, return the instrumented promise if (!isThenable(originalPromiseLike)) { @@ -292,7 +280,7 @@ export function wrapPromiseWithMethods( if (prop === 'withResponse' && typeof value === 'function') { return function wrappedWithResponse(this: unknown): unknown { const originalWithResponse = (value as (...args: unknown[]) => unknown).call(target); - return createWithResponseWrapper(originalWithResponse, instrumentedPromise, mechanismType); + return createWithResponseWrapper(originalWithResponse, instrumentedPromise); }; } diff --git a/packages/server-utils/src/ai/google-genai/index.ts b/packages/server-utils/src/ai/google-genai/index.ts index c7ec34b5b70d..7342d3a043bf 100644 --- a/packages/server-utils/src/ai/google-genai/index.ts +++ b/packages/server-utils/src/ai/google-genai/index.ts @@ -1,7 +1,6 @@ /* eslint-disable typescript-eslint/no-deprecated */ /* eslint-disable max-lines */ import { - captureException, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startSpan, @@ -289,13 +288,6 @@ function instrumentMethod( return instrumentStream(stream, span, Boolean(options.recordOutputs)) as R; } catch (error) { span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - type: 'auto.ai.google_genai', - data: { function: methodPath }, - }, - }); span.end(); throw error; } @@ -314,13 +306,11 @@ function instrumentMethod( addPrivateRequestAttributes(span, params, operationName); } + // `onError` is a no-op because the rejection is rethrown to the caller and `startSpan` already + // marks the span errored; both leading callbacks are positional and only exist to reach `onSuccess`. return handleCallbackErrors( () => target.apply(context, args), - error => { - captureException(error, { - mechanism: { handled: false, type: 'auto.ai.google_genai', data: { function: methodPath } }, - }); - }, + () => {}, () => {}, result => { // Only add response attributes for content-producing methods, not for embeddings diff --git a/packages/server-utils/src/ai/openai/index.ts b/packages/server-utils/src/ai/openai/index.ts index ded2f28d755d..83d9aceb9541 100644 --- a/packages/server-utils/src/ai/openai/index.ts +++ b/packages/server-utils/src/ai/openai/index.ts @@ -1,7 +1,6 @@ /* eslint-disable typescript-eslint/no-deprecated */ import { DEBUG_BUILD } from '../../debug-build'; import { - captureException, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startSpan, @@ -172,20 +171,13 @@ function instrumentMethod( ) as unknown as R; } catch (error) { span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - type: 'auto.ai.openai.stream', - data: { function: methodPath }, - }, - }); span.end(); throw error; } })(); }); - return wrapPromiseWithMethods(originalResult, instrumentedPromise, 'auto.ai.openai'); + return wrapPromiseWithMethods(originalResult, instrumentedPromise); } // Non-streaming @@ -199,25 +191,13 @@ function instrumentMethod( addRequestAttributes(span, params, operationName); } - return originalResult.then( - result => { - addResponseAttributes(span, result, options.recordOutputs); - return result; - }, - error => { - captureException(error, { - mechanism: { - handled: false, - type: 'auto.ai.openai', - data: { function: methodPath }, - }, - }); - throw error; - }, - ); + return originalResult.then(result => { + addResponseAttributes(span, result, options.recordOutputs); + return result; + }); }); - return wrapPromiseWithMethods(originalResult, instrumentedPromise, 'auto.ai.openai'); + return wrapPromiseWithMethods(originalResult, instrumentedPromise); }; } diff --git a/packages/server-utils/test/ai/lib/tracing/ai/utils.test.ts b/packages/server-utils/test/ai/lib/tracing/ai/utils.test.ts index e8a4640797e9..6371a8129a73 100644 --- a/packages/server-utils/test/ai/lib/tracing/ai/utils.test.ts +++ b/packages/server-utils/test/ai/lib/tracing/ai/utils.test.ts @@ -86,7 +86,7 @@ describe('wrapPromiseWithMethods', () => { request_id: 'req_123', }); const instrumented = Promise.resolve('instrumented-data'); - const wrapped = wrapPromiseWithMethods(original, instrumented, 'auto.ai.test'); + const wrapped = wrapPromiseWithMethods(original, instrumented); const result = await wrapped; expect(result).toBe('instrumented-data'); @@ -98,7 +98,7 @@ describe('wrapPromiseWithMethods', () => { request_id: 'req_123', }); const instrumented = Promise.resolve('instrumented-data'); - const wrapped = wrapPromiseWithMethods(original, instrumented, 'auto.ai.test'); + const wrapped = wrapPromiseWithMethods(original, instrumented); const withResponseResult = await (wrapped as typeof original).withResponse(); expect(withResponseResult).toEqual({ @@ -115,7 +115,7 @@ describe('wrapPromiseWithMethods', () => { request_id: 'req_123', }); const instrumented = Promise.resolve('instrumented-data'); - const wrapped = wrapPromiseWithMethods(original, instrumented, 'auto.ai.test'); + const wrapped = wrapPromiseWithMethods(original, instrumented); const response = await (wrapped as typeof original).asResponse(); expect(response).toBe(mockResponse); @@ -124,7 +124,7 @@ describe('wrapPromiseWithMethods', () => { it('returns instrumentedPromise when original is not thenable', async () => { const instrumented = Promise.resolve('instrumented-data'); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const wrapped = wrapPromiseWithMethods(null as any, instrumented, 'auto.ai.test'); + const wrapped = wrapPromiseWithMethods(null as any, instrumented); const result = await wrapped; expect(result).toBe('instrumented-data'); @@ -136,7 +136,7 @@ describe('wrapPromiseWithMethods', () => { request_id: 'req_123', }); const instrumented = Promise.reject(new Error('instrumented-error')); - const wrapped = wrapPromiseWithMethods(original, instrumented, 'auto.ai.test'); + const wrapped = wrapPromiseWithMethods(original, instrumented); await expect(wrapped).rejects.toThrow('instrumented-error'); }); diff --git a/packages/server-utils/test/ai/tracing/ai-client-error-capture.test.ts b/packages/server-utils/test/ai/tracing/ai-client-error-capture.test.ts new file mode 100644 index 000000000000..31572676d70b --- /dev/null +++ b/packages/server-utils/test/ai/tracing/ai-client-error-capture.test.ts @@ -0,0 +1,132 @@ +import type * as SentryCore from '@sentry/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@sentry/core', async importOriginal => { + const actual = (await importOriginal()) as typeof SentryCore; + return { ...actual, captureException: vi.fn() }; +}); + +import { captureException } from '@sentry/core'; +import { instrumentAnthropicAiClient } from '../../../src/ai/anthropic-ai'; +import { instrumentGoogleGenAIClient } from '../../../src/ai/google-genai'; +import { instrumentOpenAiClient } from '../../../src/ai/openai'; + +const upstreamError = new Error('Rate limit exceeded'); + +/** Rejects on the first call and resolves afterwards, like a transient provider failure. */ +function transientFailure(response: unknown): (...args: unknown[]) => Promise { + let call = 0; + return () => (call++ === 0 ? Promise.reject(upstreamError) : Promise.resolve(response)); +} + +const openAiResponse = { + id: 'chatcmpl-1', + model: 'gpt-4o-mini', + object: 'chat.completion', + choices: [{ index: 0, message: { role: 'assistant', content: 'ok' }, finish_reason: 'stop' }], +}; + +const anthropicResponse = { id: 'msg_1', model: 'claude-sonnet-4-5', type: 'message', content: [], usage: {} }; + +const googleResponse = { modelVersion: 'gemini-2.0-flash', candidates: [], usageMetadata: {} }; + +describe('AI client instrumentation error handling', () => { + beforeEach(() => { + vi.mocked(captureException).mockClear(); + }); + + describe.each([ + { + provider: 'openai', + instrument: () => { + const create = transientFailure(openAiResponse); + const client = instrumentOpenAiClient({ chat: { completions: { create } } }); + return () => client.chat.completions.create({ model: 'gpt-4o-mini', messages: [] }); + }, + }, + { + provider: 'openai (streaming)', + instrument: () => { + const create = transientFailure(openAiResponse); + const client = instrumentOpenAiClient({ chat: { completions: { create } } }); + return () => client.chat.completions.create({ model: 'gpt-4o-mini', messages: [], stream: true }); + }, + }, + { + provider: 'anthropic', + instrument: () => { + const create = transientFailure(anthropicResponse); + const client = instrumentAnthropicAiClient({ messages: { create } }); + return () => client.messages.create({ model: 'claude-sonnet-4-5', messages: [] }); + }, + }, + { + provider: 'anthropic (streaming)', + instrument: () => { + const create = transientFailure(anthropicResponse); + const client = instrumentAnthropicAiClient({ messages: { create } }); + return () => client.messages.create({ model: 'claude-sonnet-4-5', messages: [], stream: true }); + }, + }, + { + provider: 'google-genai', + instrument: () => { + const generateContent = transientFailure(googleResponse); + const client = instrumentGoogleGenAIClient({ models: { generateContent } }); + return () => client.models.generateContent({ model: 'gemini-2.0-flash', contents: 'hi' }); + }, + }, + { + provider: 'google-genai (streaming)', + instrument: () => { + const generateContentStream = transientFailure(googleResponse); + const client = instrumentGoogleGenAIClient({ models: { generateContentStream } }); + return () => client.models.generateContentStream({ model: 'gemini-2.0-flash', contents: 'hi' }); + }, + }, + ])('$provider', ({ instrument }) => { + it('rethrows the original error without capturing it', async () => { + const call = instrument(); + + await expect(call()).rejects.toBe(upstreamError); + expect(captureException).not.toHaveBeenCalled(); + }); + + it('reports nothing when a retry recovers the call', async () => { + const call = instrument(); + + await expect(call()).rejects.toBe(upstreamError); + await expect(call()).resolves.toBeDefined(); + + expect(captureException).not.toHaveBeenCalled(); + }); + + it('reports nothing per attempt when every retry fails', async () => { + // `transientFailure` only rejects its first call, so re-instrument per attempt to model + // a caller that retries the same failing request three times. + for (let attempt = 0; attempt < 3; attempt++) { + await expect(instrument()()).rejects.toBe(upstreamError); + } + + expect(captureException).not.toHaveBeenCalled(); + }); + }); + + it('does not capture when a rejection is observed through .withResponse()', async () => { + const create = (..._args: unknown[]): Promise => { + const promise = Promise.reject(upstreamError) as Promise & { withResponse: () => Promise }; + promise.withResponse = () => Promise.reject(upstreamError); + return promise; + }; + const client = instrumentOpenAiClient({ chat: { completions: { create } } }); + + const result = client.chat.completions.create({ model: 'gpt-4o-mini', messages: [] }) as Promise & { + withResponse: () => Promise; + }; + + await expect(result).rejects.toBe(upstreamError); + await expect(result.withResponse()).rejects.toBe(upstreamError); + + expect(captureException).not.toHaveBeenCalled(); + }); +});