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

closes #350: errorCode and exceptionClass to be reported always with … #369

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

ivansenic
Copy link
Contributor

@ivansenic ivansenic commented Apr 18, 2023

What this PR does:
As discussed in the #350 (comment) I pushed the implementation that fixes errorCode missing. We still need to agree on the handling of the cause and if we do want the error array to include the cause..

Which issue(s) this PR fixes:
Fixes #350

Checklist

  • Agree on cause handling

@ivansenic ivansenic requested a review from a team as a code owner April 18, 2023 13:58
@@ -59,6 +56,12 @@ public CommandResult get() {
}
}

public CommandResult.Error getCommandResultError(String message) {
Map<String, Object> fields =
Map.of("errorCode", errorCode.name(), "exceptionClass", this.getClass().getSimpleName());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the JsonApiException was not including the exceptionClass before so I added it, I guess this is also helpful information..

fields.put("errorCode", jae.getErrorCode().name());
}
return new CommandResult.Error(message, fields);
return ThrowableToErrorMapper.getMapperWithMessageFunction().apply(throwable, message);
Copy link
Contributor Author

@ivansenic ivansenic Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here I adapted this, the ExceptionUtil should not know how are CommandResult.Error instances created, cause we have a single place for that.. Adapted for custom message..

@ivansenic ivansenic force-pushed the ise-350-error-chain branch from fb302f2 to 661a067 Compare April 18, 2023 16:41
@ivansenic ivansenic merged commit 19b3f23 into main Apr 18, 2023
@ivansenic ivansenic deleted the ise-350-error-chain branch April 18, 2023 17:23
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors in response
3 participants