Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

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

Open
machineghost opened this issue Mar 18, 2020 · 4 comments

Comments

@machineghost
Copy link

machineghost commented Mar 18, 2020

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
Copy link
Author

machineghost commented Mar 18, 2020

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 added a commit to machineghost/express-graphql that referenced this issue Mar 18, 2020
@machineghost
Copy link
Author

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.

@danielrearden
Copy link
Collaborator

@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?

@machineghost
Copy link
Author

@danielrearden Fixed, sorry about that (#612)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants