-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(node): Ensure tool errors for vercelAiIntegration
have correct trace connected
#17132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5b84119
51397fa
63e07ce
d97451c
8100e99
c6caf49
1f2443f
59e546e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ Sentry.init({ | |
dsn: 'https://[email protected]/1337', | ||
release: '1.0', | ||
transport: loggingTransport, | ||
tracesSampleRate: 1, | ||
tracesSampleRate: 0, | ||
}); | ||
|
||
import express from 'express'; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
release: '1.0', | ||
tracesSampleRate: 1, | ||
transport: loggingTransport, | ||
}); | ||
|
||
import express from 'express'; | ||
|
||
const app = express(); | ||
|
||
app.get('/test/express/:id', req => { | ||
throw new Error(`test_error with id ${req.params.id}`); | ||
}); | ||
|
||
Sentry.setupExpressErrorHandler(app); | ||
|
||
startExpressServerAndSendPortToRunner(app); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { afterAll, expect, test } from 'vitest'; | ||
import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; | ||
|
||
afterAll(() => { | ||
cleanupChildProcesses(); | ||
}); | ||
|
||
test('should capture and send Express controller error with txn name if tracesSampleRate is 1', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. noticed this test was missing, just adding for completeness sake. |
||
const runner = createRunner(__dirname, 'server.ts') | ||
.expect({ | ||
transaction: { | ||
transaction: 'GET /test/express/:id', | ||
}, | ||
}) | ||
.expect({ | ||
event: { | ||
exception: { | ||
values: [ | ||
{ | ||
mechanism: { | ||
type: 'middleware', | ||
handled: false, | ||
}, | ||
type: 'Error', | ||
value: 'test_error with id 123', | ||
stacktrace: { | ||
frames: expect.arrayContaining([ | ||
expect.objectContaining({ | ||
function: expect.any(String), | ||
lineno: expect.any(Number), | ||
colno: expect.any(Number), | ||
}), | ||
]), | ||
}, | ||
}, | ||
], | ||
}, | ||
transaction: 'GET /test/express/:id', | ||
}, | ||
}) | ||
.start(); | ||
runner.makeRequest('get', '/test/express/123', { expectError: true }); | ||
await runner.completed(); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
release: '1.0', | ||
tracesSampleRate: 1, | ||
transport: loggingTransport, | ||
}); | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
recordSpan(async () => { | ||
// eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
doSomething(); | ||
// eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
doSomethingWithError(); | ||
}); | ||
|
||
async function doSomething(): Promise<void> { | ||
return Promise.resolve(); | ||
} | ||
|
||
async function doSomethingWithError(): Promise<void> { | ||
await new Promise(resolve => setTimeout(resolve, 100)); | ||
throw new Error('test error'); | ||
} | ||
|
||
// Duplicating some code from vercel-ai to verify how things work in more complex/weird scenarios | ||
function recordSpan(fn: (span: unknown) => Promise<void>): Promise<void> { | ||
return Sentry.startSpanManual({ name: 'test-span' }, async span => { | ||
try { | ||
const result = await fn(span); | ||
span.end(); | ||
return result; | ||
} catch (error) { | ||
try { | ||
span.setStatus({ code: 2 }); | ||
} finally { | ||
// always stop the span when there is an error: | ||
span.end(); | ||
} | ||
|
||
throw error; | ||
} | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
release: '1.0', | ||
tracesSampleRate: 1, | ||
transport: loggingTransport, | ||
}); | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
Sentry.startSpan({ name: 'test-span' }, async () => { | ||
throw new Error('test error'); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leftover that should not be needed anymore |
||
integrations: [Sentry.vercelAIIntegration()], | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :O |
||
import { generateText } from 'ai'; | ||
import { MockLanguageModelV1 } from 'ai/test'; | ||
import express from 'express'; | ||
import { z } from 'zod'; | ||
|
||
const app = express(); | ||
|
||
app.get('/test/error-in-tool', async (_req, res, next) => { | ||
Sentry.setTag('test-tag', 'test-value'); | ||
|
||
try { | ||
await generateText({ | ||
model: new MockLanguageModelV1({ | ||
doGenerate: async () => ({ | ||
rawCall: { rawPrompt: null, rawSettings: {} }, | ||
finishReason: 'tool-calls', | ||
usage: { promptTokens: 15, completionTokens: 25 }, | ||
text: 'Tool call completed!', | ||
toolCalls: [ | ||
{ | ||
toolCallType: 'function', | ||
toolCallId: 'call-1', | ||
toolName: 'getWeather', | ||
args: '{ "location": "San Francisco" }', | ||
}, | ||
], | ||
}), | ||
}), | ||
tools: { | ||
getWeather: { | ||
parameters: z.object({ location: z.string() }), | ||
execute: async () => { | ||
throw new Error('Error in tool'); | ||
}, | ||
}, | ||
}, | ||
prompt: 'What is the weather in San Francisco?', | ||
}); | ||
} catch (error) { | ||
next(error); | ||
return; | ||
} | ||
|
||
res.send({ message: 'OK' }); | ||
}); | ||
Sentry.setupExpressErrorHandler(app); | ||
|
||
startExpressServerAndSendPortToRunner(app); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import * as Sentry from '@sentry/node'; | ||
import { generateText } from 'ai'; | ||
import { MockLanguageModelV1 } from 'ai/test'; | ||
import { z } from 'zod'; | ||
|
||
async function run() { | ||
Sentry.setTag('test-tag', 'test-value'); | ||
|
||
await Sentry.startSpan({ op: 'function', name: 'main' }, async () => { | ||
await generateText({ | ||
model: new MockLanguageModelV1({ | ||
doGenerate: async () => ({ | ||
rawCall: { rawPrompt: null, rawSettings: {} }, | ||
finishReason: 'tool-calls', | ||
usage: { promptTokens: 15, completionTokens: 25 }, | ||
text: 'Tool call completed!', | ||
toolCalls: [ | ||
{ | ||
toolCallType: 'function', | ||
toolCallId: 'call-1', | ||
toolName: 'getWeather', | ||
args: '{ "location": "San Francisco" }', | ||
}, | ||
], | ||
}), | ||
}), | ||
tools: { | ||
getWeather: { | ||
parameters: z.object({ location: z.string() }), | ||
execute: async () => { | ||
throw new Error('Error in tool'); | ||
}, | ||
}, | ||
}, | ||
prompt: 'What is the weather in San Francisco?', | ||
}); | ||
}); | ||
} | ||
|
||
run(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was actually not testing what the test said it would ^^