-
-
Notifications
You must be signed in to change notification settings - Fork 547
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
Fix bug with errors thrown in the on_parse_* extension hooks #1324
Conversation
Before they would have been swallowed and just returned in the results object. This change means that they get propagated up as you would expect.
Thanks for adding the Here's a preview of the changelog: Fix bug where errors thrown in the on_parse_* extension hooks were being Here's the preview release card for twitter: Here's the tweet text:
|
Codecov Report
@@ Coverage Diff @@
## main #1324 +/- ##
=======================================
Coverage 98.04% 98.04%
=======================================
Files 105 105
Lines 3786 3786
Branches 547 547
=======================================
Hits 3712 3712
Misses 41 41
Partials 33 33 |
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.
Good find! Fix and tests look sensible to me 👍
except Exception as error: # pragma: no cover | ||
error = GraphQLError(str(error), original_error=error) | ||
|
||
execution_context.errors = [error] | ||
return ExecutionResult( | ||
data=None, | ||
errors=[error], | ||
extensions=await extensions_runner.get_extensions_results(), | ||
) |
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.
Noticed while investigating #3204:
Looks like this block should not have been moved or kept at all. Now it's catching Exception
s raised by parse_document
, which there shouldn't be any.
Before it was moved, it used to catch errors raised inside extensions during the parsing step and add those to the execution result. Such errors are usually progamming errors which we might prefer to cause an internal server error instead.
Description
Before they would have been swallowed and just returned in the results
object. This change means that they get propagated up as you would
expect.
Types of Changes
Issues Fixed or Closed by This PR
Checklist