GraphQLError: Fixed originalError.extensions overriding extensions argument to constructor

[klippx]

When upgrading to latest version of graphql I discovered a regression:

We are enriching an existing graphql error and the implementation has changed in a way that is different between ^15.6.1 and 15.x.x

This test runs fine on v15.6.1 but needs the fix I provided to work on 15.x.x

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Mathias Klippinge (f6329e9)

[yaacovCR]

Reference

graphql-js/src/error/GraphQLError.js

Line 130 in dcf3751

if (_extensions == null && originalError != null) {

[IvanGoncharov]

@yaacovCR @dotansimha @saihaj Thanks for timely review 👍

@klippx Fix itself is good, I just changed the test to be more in line with our test style.
See my commit for details, but the main point is that:

• We don't use JSON.stringify for snapshots. The only exceptions are tests testing compatibility with JSON.stringify.
• originalError is optional so you don't need to specify const original = new Error('original');
• if you have multiple instances of something try to find more descriptive names for it
• you can merge many small and related test cases into one with multiple checks if it tests the same behavior.

Hope this feedback helps you with your future contributions.

[IvanGoncharov]

For future context, regression came from here:
https://github.com/graphql/graphql-js/pull/3332/files#diff-eb4c8bfdacd520fb9f1f9004c125a2ba86871f645fd25d4a7c1c7d8b6af7e36eL129-L134

[IvanGoncharov]

Released as https://github.com/graphql/graphql-js/releases/tag/v15.7.2

[klippx]

Thanks @IvanGoncharov, I agree that your review commit is a more elegant way of testing things. And I appreciate that you did the review comment for me! 🙏 🙇