fix: return correct originalError after normalize for deprecated GraphQLError
In graphql-js v16 property becomes nested error.originalError.originalError in toNormalizedOptions, but should be just error.originalError.
Added condition to check if error instance of GraphQLError then just return args[4].originalError.
Issue happens in executeFieldsSerially.
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: marko-zub / name: marko (befe1abd6a14493df8b3c5fff1928846a98282c8)
@yaacovCR please approve github workflow. Lint errors has been fixed and added test. Thanks
@yaacovCR What next steps to merge current PR?
Hi @marko-zub --
As far as I follow here, this PR is asserting that when the GraphQLError constructor is supplied an object of type GraphQLError as the originalError, the new GraphQLError object's originalError property should point to the originalError within the supplied GraphQLError property.
Whereas the existing behavior (as far as I can tell) in both v15, v16, v17-alpha all assume that the new GraphQLError object's originalError property should point to the intermediate GraphQLError object which will have its own originalError property.
I suppose the question is whether the interpretation of the originalError should be more like rootError (this PR) or the immediatelyCausativeError (existing behavior).
Considering that the latter option preserves more data, I guess I am in favor. It is also similar to the new cause property for Error objects. In fact, perhaps it could eventually be superseded by this property.
Links: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#differentiate_between_similar_errors https://github.com/tc39/proposal-error-cause
But from your text above, it seems like you believe there is something that changed with v16 or something v16-specific that requires fixing/changing?
@yaacovCR
Issue specific for v16
we rely on error.originalError property (in v15 it works correctly), but in v16 after execution executeFieldsSerially and pass GraphQLError in locatedError we receive nested error.originalError.originalError property it's seems that reason is in toNormalizedOptions
Please check test which demonstrate the possible reason of issue.
I don’t yet see the code change resulting in a change of behavior. Are you able to set up a small reproduction that works in version 15 and brakes with version 16?
Closing for now, please feel free to reopen with more information!