Improve API error handling #730
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We would like to inspect database errors to match specific constraint errors to specific user facing errors, for example. (Otherwise we would need to duplicate that logic in Rust.) However, before this commit that was not necessary as our `Tx` helper would log any DB error immediately and later, the `handle_api` would check if any DB error occurred and reply with 500 then. That's not very useful. With this commit, we will roll back the DB transaction if any `FieldError`s happen during query execution. To find that out, we had to use a lower level Juniper API and duplicate a bit of Juniper code. However, we don't always reply 500 in that case, but reply with those exact field errors, but no data.
The data does not come from the DB, so rolling back the transaction does not potentially invalidate the user data. Thus we can safely return it. It is actually useful as it's required to show the user info in the header.
The previous comment was wrong: some fields can actually be omitted, for example when they weren't requested or when that specific field returned an error.
4 tasks
|
Ole had a quick look at this PR and didn't find anything problematic. I asked Julian to review this as well, which he will do some time in the future. But to move along with #727, we decided to merge this already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main change is in the first commit. See its message for more information and motivation. Required for #727