Skip to content
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

check response and throwable #565

Merged
merged 2 commits into from Dec 6, 2021
Merged

Conversation

tbradellis
Copy link
Contributor

@tbradellis tbradellis commented Nov 18, 2021

This address the issue described here:
#376

Setting a expected_status_code will not mark an error as expected when there is an exception in the transaction.

The problem starts here with markedExpected:
newrelic-java-agent/newrelic-agent/src/main/java/com/newrelic/agent/errors/ErrorServiceImpl.java

We only check whether it's expected on the Throwable and do not consult the expected status codes....As @meiao hinted on ThrowableError and HttpTracedError.

Essentially, we should check if the throwable OR a status code is expected and mark it as such. This change does that.

Overview

checks both response status and throwable for if they were configured to be expected. Previously, we were not checking the response status in this code path, so there are cases where a response status can be configured expected, but the TracedError will not get created as expected.

Related Github Issue

#376

Checks

[ NA] Are your contributions backwards compatible with relevant frameworks and APIs?
[ no] Does your code contain any breaking changes? Please describe.
[ no] Does your code introduce any new dependencies? Please describe.

@kford-newrelic kford-newrelic added this to Triage in Java Engineering Board via automation Nov 29, 2021
@kford-newrelic kford-newrelic moved this from Triage to To do in Java Engineering Board Nov 29, 2021
@kford-newrelic kford-newrelic moved this from To do to Needs Review in Java Engineering Board Dec 2, 2021
@tbradellis tbradellis self-assigned this Dec 2, 2021
@tbradellis tbradellis merged commit ca22aca into main Dec 6, 2021
Java Engineering Board automation moved this from Needs Review to Done Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants