- Notifications
You must be signed in to change notification settings - Fork12
#127 Improve Error Handling for Failed DefectDojo API Requests#128
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…questsTwo Problems:1. We do not pass consequently the causing exception to our own custom exception.2. We do not catch all possible REST client runtime exceptions.Fixed by consequent logging and passing ofthe origin exception ascause of our own excpetion. Also catching a more generic exceptiontype totach all possible errors.Signed-off-by: Sven Strittmatter <sven.strittmatter@iteratec.com>
| return response.getBody(); | ||
| try { | ||
| ResponseEntity<T> response = restTemplate.exchange( |
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.
Did you intentionally only add this for the GET requests?
The error handling for the other http methods seems to be the same
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.
🤔No. This means that any error handling is missing on non-GET bc I simply greppedHttpClientErrorException 😬
| return response.getBody(); | ||
| } catch (RestClientException e) { | ||
| log.error("Exception while getting data: {}", e.getMessage()); |
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.
Whats in a message for a request client exception?
Is that enough for users to understand what request failed? Does it include the http method and url of the request?
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.
That's a common pattern in Java that you simply log the exception message, if the exception is rethrown with the origin exception as cause because the full exception handling is done where the rethrown exception is catched. It's quite debatable if this log message is really necessary.
The conatenation will be done always, also if debug level is notlogged. So we should alwyas use parameterized log statements.Signed-off-by: Sven Strittmatter <sven.strittmatter@iteratec.com>
…P MethodsIn previous commits only GET requests were covered with properexception handling.This adds rethrow of our own custom exception with the originatingerror as cause, like already implemented for GET requests.Signed-off-by: Sven Strittmatter <sven.strittmatter@iteratec.com>
c2daf60 intosecureCodeBox:mainUh oh!
There was an error while loading.Please reload this page.
Two Problems:
Fixed by consequent logging and passing ofthe origin exception as cause of our own excpetion. Also catching a more generic exception type totach all possible errors.