feat: Ensure API errors are tracked #1880
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1880 +/- ##
==========================================
+ Coverage 96.87% 96.88% +0.01%
==========================================
Files 209 210 +1
Lines 39635 39698 +63
==========================================
+ Hits 38397 38463 +66
+ Misses 1238 1235 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
lib/llm-events/openai/llm-error.js
Outdated
| * Represents an error object, to be tracked via `agent.errors`, that is the | ||
| * result of some error returned from the OpenAI API. | ||
| */ | ||
| class OpenAiLlmError extends Error { |
There was a problem hiding this comment.
This should just be a class, not inheriting from Error. When you call agent.errors.add the signature is transaction, err, customAttrs. This class should be providing all the custom attrs. Since the NR exceptions do not handle error.code and error.param those will be properties on this class.
This PR is to resolve newrelic#1845.
jsumners-nr
force-pushed
the
issue-1845
branch
2 times, most recently
from
200ad00
to
7dab013
Compare
jsumners-nr
force-pushed
the
issue-1845
branch
from
7dab013
to
9c155c7
Compare
| @@ -35,6 +36,7 @@ function shouldSkipInstrumentation(config, shim) { | |||
| return semver.lt(pkgVersion, MIN_VERSION) | |||
| } | |||
|
|
|||
| // eslint-disable-next-line sonarjs/cognitive-complexity | |||
There was a problem hiding this comment.
i noticed this too. I think it's because we're defining all the functions within the module.exports. We try not to add to the cognitive complexity but we can address later
| */ | ||
| function recordChatCompletionMessages({ segment, request, response }) { | ||
| function recordChatCompletionMessages({ segment, request, response, hasError = false }) { |
There was a problem hiding this comment.
What is your suggestion?
There was a problem hiding this comment.
just calling out the arg coming in is diff than it going into the event creation
There was a problem hiding this comment.
In my mind there's a difference between the two, but I'm totally open to changing it one way or the other. Which would you prefer?
This PR is to resolve #1845.