-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(node): Preserve synchronous return behavior for streamText and other methods for AI #17580
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 3 commits
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 |
|---|---|---|
|
|
@@ -71,7 +71,7 @@ function isToolError(obj: unknown): obj is ToolError { | |
| * Check for tool errors in the result and capture them | ||
| * Tool errors are not rejected in Vercel V5, it is added as metadata to the result content | ||
| */ | ||
| function checkResultForToolErrors(result: unknown | Promise<unknown>): void { | ||
| function checkResultForToolErrors(result: unknown): void { | ||
| if (typeof result !== 'object' || result === null || !('content' in result)) { | ||
| return; | ||
| } | ||
|
|
@@ -132,6 +132,15 @@ function checkResultForToolErrors(result: unknown | Promise<unknown>): void { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the given value is a promise like object. | ||
| * @param value The value to check. | ||
| * @returns True if the value is a promise like object, false otherwise. | ||
| */ | ||
| function isPromiseLike<T = unknown>(value: unknown): value is PromiseLike<T> { | ||
| return !!value && typeof value === 'object' && typeof (value as { then?: unknown }).then === 'function'; | ||
| } | ||
|
|
||
| /** | ||
| * Determines whether to record inputs and outputs for Vercel AI telemetry based on the configuration hierarchy. | ||
| * | ||
|
|
@@ -236,13 +245,18 @@ export class SentryVercelAiInstrumentation extends InstrumentationBase { | |
| }; | ||
|
|
||
| return handleCallbackErrors( | ||
| async () => { | ||
| () => { | ||
| // @ts-expect-error we know that the method exists | ||
| const result = await originalMethod.apply(this, args); | ||
| const result = originalMethod.apply(this, args); | ||
|
|
||
| // Tool errors are not rejected in Vercel V5, it is added as metadata to the result content | ||
| checkResultForToolErrors(result); | ||
| if (isPromiseLike(result)) { | ||
| // check for tool errors when the promise resolves, keep the original promise identity | ||
| result.then(checkResultForToolErrors, () => {}); | ||
|
Member
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. hmm, I think we need to re-throw here or something like this, as otherwise this could swallow unhandled promise rejections, possibly...? Can this be? This is always hard tor reason about for me...
Member
Author
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. oh yah it confuses me too! I just tested this though and the error should bubble up to user as expected
RulaKhaled marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return result; | ||
| } | ||
|
|
||
| // check for tool errors when the result is synchronous | ||
| checkResultForToolErrors(result); | ||
| return result; | ||
| }, | ||
| error => { | ||
|
|
||
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.
instead of this, you could also use the existing
isThenableutility!