fix(core): Align error span status message with core SpanStatusType for langchain/google-genai#19863
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Deps
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } | ||
|
|
||
| const ANTHROPIC_ERROR_TYPE_TO_SPAN_STATUS: Record<string, string> = { | ||
| invalid_request_error: 'invalid_argument', |
There was a problem hiding this comment.
that doesn't exist in SpanStatusType
There was a problem hiding this comment.
Aligning the status here to our span status values sounds like a good call! I had some typing suggestions and I agree with bugbot that a unit test or two would be good :)
Side-note: Long term, the new span protocol will only accept ok and error as span status. So we can merge this for today's transactions but we might be able to get rid of this code entirely at a later point.
| }); | ||
| } | ||
|
|
||
| const ANTHROPIC_ERROR_TYPE_TO_SPAN_STATUS: Record<string, string> = { |
There was a problem hiding this comment.
l: Can we type this as Record<string, SpanStatus>? (actual questions, I don't know the full type by heart. No strong feelings though, I was just thinking of some additional type safety)
There was a problem hiding this comment.
right string is a bit generic, we can type it as SpanStatusType
| * Map an Anthropic API error type to a SpanStatusType value. | ||
| * @see https://docs.anthropic.com/en/api/errors#error-shapes | ||
| */ | ||
| export function mapAnthropicErrorToStatusMessage(errorType: string | undefined): string { |
There was a problem hiding this comment.
l: same question here, can we return a SpanStatus instead of a string?
There was a problem hiding this comment.
same here I'll type it as SpanStatusType
4 participants
In case of exceptions we should set the span status to a known error value from our core
SpanStatusType.Closes #19862