-
Notifications
You must be signed in to change notification settings - Fork 2k
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: invalid originalError propagation in custom scalars #3837
fix: invalid originalError propagation in custom scalars #3837
Conversation
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@stenreijers maybe push an empty commit to retigger the CI ? |
/easycla |
This makes sense as a problem to me and also as the solution. Another approach could be to introduce a generic cloneError utility that does an appropriate deep clone => but I think that rather than attempting the deep clone, it makes more sense to preserve the error hierarchy appropriately... |
@stenreijers Thanks for fixing it. graphql-js/src/utilities/coerceValue.js Lines 62 to 82 in 18371cb
graphql-js/src/utilities/coerceValue.js Lines 214 to 236 in 18371cb
Can you please backport this fix to https://github.com/graphql/graphql-js/tree/16.x.x ? |
Please see: |
* fix: custom scalar original error propagation (backport graphql/graphql-js#3837; graphql/graphql-js@076972e) * Update packages/executor/src/execution/__tests__/variables-test.ts Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com> * okok --------- Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
Errors thrown in the
parseValue
function for custom scalars are not propagated correctly using theoriginalError
variable of the GraphQLError on invalid input. As a result, error codes from theextensions.code
are not propagated correctly.The fix is simple: Replace
error.orginalError
witherror
, sinceerror.orginalError
does not exist yet.see:
graphql-js/src/execution/values.ts
Line 134 in 8be83d8
I have added test-cases to avoid this problem in the future.