Skip to content
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

Incorrect error handling behavior #1719

Closed
steelbrain opened this issue Jul 2, 2020 · 4 comments
Closed

Incorrect error handling behavior #1719

steelbrain opened this issue Jul 2, 2020 · 4 comments

Comments

@steelbrain
Copy link

steelbrain commented Jul 2, 2020

Context:

Here's what the schema looks like in this scenario:

type Query {
  viewer: Viewer
}
type Viewer {
  id: ID!
  institutions: [Institution]!
}
type Institution {
  id: ID!
  name: String!
}

We were running into a bug where ANY error in Viewer.institutions resolver would ALWAYS throw out a Error: Cannot return null for non-nullable field User.id for the GQL request which makes absolutely no sense at all.

Investigation:

Upon some rigorous digging, here's what happens:

Apollo executes the GraphQL schema -> GraphQL resolves viewer (with a wrapped resolveFn coming from this package). The wrapper calls delegateRequest, the result trace so far is correct.

Then we get to this part, transformer.transformResult converts

{
  data: { viewer: null },
  errors: [...Institution Error...]
}

into

{
  institution: ...InstitutionError...
}

This is not what the caller expects, so tries to resolve id from {institution: Error}. I think this is incorrect behavior. I'm not familiar enough with internals to suggest a proper fix except handleNull needs to handle this.

@yaacovCR
Copy link
Collaborator

yaacovCR commented Jul 2, 2020

Can you create a minimal reproduction?

Thanks!

@yaacovCR
Copy link
Collaborator

yaacovCR commented Jul 2, 2020

Is this possible related to #1047

@steelbrain
Copy link
Author

It's likely related, I'll create a minimal reproduction tomorrow. Thanks!

@steelbrain
Copy link
Author

steelbrain commented Jul 5, 2020

Unable to repro in an isolated environment. It's possible that some of the deep dependency in production app is using an older version of graphql-tools.

Closing this until I have more information. Thank you

Edit: Update graphql-tools in all deep deps to latest version, still getting the error. Sigh. I wish it was easier to isolate this into a repro.

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

No branches or pull requests

2 participants