refactor: convert bedrock's LlmError to use LlmErrorMessage instead
#3567
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3567 +/- ##
==========================================
- Coverage 97.80% 97.68% -0.13%
==========================================
Files 428 427 -1
Lines 56592 56585 -7
Branches 1 1
==========================================
- Hits 55351 55273 -78
- Misses 1241 1312 +71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @param {LlmTool} [params.tool] Details about the tool event if it was a tool event. | ||
| * @param {boolean} [params.useNameAsCode] defaults to false, only Bedrock sets it to true so far | ||
| */ | ||
| constructor({ response, cause, summary, embedding, vectorsearch, tool } = {}) { |
There was a problem hiding this comment.
I don't think I realized that this class is overloaded. We handle specific assignment of attributes depending on params and/or further processing. It seems like most LLMs will either use keys on response or error(cause) to assign attributes but in the case of gemini it parses error and in case of bedrock is uses the error.name for the code instead of error.code
There was a problem hiding this comment.
I was thinking of maybe having error message classes that inherit from LlmErrorMessage for Bedrock and Gemini. I was planning to do in the AIM subscriber refactor.
There was a problem hiding this comment.
for sure, that's what I was referring to. it can wait until then
amychisholm03
force-pushed
the
llm-error-refactor
branch
from
7bf6081 to
8a3279b
Compare
github-project-automation
bot
moved this from Needs PR Review
to Done: Issues recently completed
in Node.js Engineering Board
2 participants
Description
AWS Bedrock instrumentation utilizes a unique
LlmErrorclass. It should use theLlmErrorMessageclass all other LLM packages use instead.How to Test