express-graphql icon indicating copy to clipboard operation
express-graphql copied to clipboard

How Can I Use customFormatErrorFn to Get What My Resolver Returned (When it Doesn't Match the Schema and Causes an Error)

Open machineghost opened this issue 4 years ago • 4 comments

I'm not sure if this is a documentation bug or a feature request. What I'm looking for is simple: when I have (say) a field foo: Foo where Foo has a required sub-field ... and I return an object from my resolver for foo which doesn't have that field ... I want to see the object that my resolver returned (the one with the null field).

However, from the documented fields there doesn't appear to be any way to access the value that was returned from the resolver.

P.S. I should note that the completeValueCatchingError in graphql itself does have a result property with this information, but it somehow seems to get dropped in this library (or again, maybe just isn't documented).

machineghost avatar Mar 18 '20 17:03 machineghost

Actually, it looks like this would be super simple to fix (but it does look like a change is needed, not just documentation).

On line 371 of index.js (https://github.com/graphql/express-graphql/blob/34f4b210dece533d94f642e3fe2b892c02c0d556/src/index.js) we can see that express-graphql has the results, and simply doesn't give them to the error formatter:

    // Format any encountered errors.
    if (result && result.errors) {
      (result: any).errors = result.errors.map(formatErrorFn);
    }

An extremely simple (one-line) fix could remedy this, by just adding result.data (ie. the result from the resolver) to the error before giving it to the formatter:

    if (result && result.errors) {
      (result: any).errors = result.errors.map(err => ({...err, result: result.data}));
      (result: any).errors = result.errors.map(formatErrorFn);
    }

I'll happily submit a PR for this (along with a documentation update) if a maintainer says they'll accept it.

machineghost avatar Mar 18 '20 17:03 machineghost

Actually, this was so simple I decided to just make the PR anyway.

I'm happy to modify as needed, but since I can't imagine this would hurt anything, and would provide useful debugging information (plus slightly improved documentation), I hope we can get it accepted in some form.

machineghost avatar Mar 18 '20 17:03 machineghost

@machineghost Thanks for contributing. It looks like a PR was never opened to merge this change. Could you rebase your branch and open a PR?

danielrearden avatar Jun 01 '20 00:06 danielrearden

@danielrearden Fixed, sorry about that (https://github.com/graphql/express-graphql/pull/612)

machineghost avatar Jun 01 '20 15:06 machineghost