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

mergeSchemas does not report correct NonNullable field errors #1047

Closed
mikebm opened this issue Jan 22, 2019 · 3 comments
Closed

mergeSchemas does not report correct NonNullable field errors #1047

mikebm opened this issue Jan 22, 2019 · 3 comments
Labels

Comments

@mikebm
Copy link

mikebm commented Jan 22, 2019

When calling a resolver that returns an item that contains a list, and that list type has a NonNullable field, yet the data getting sent back is null, then the wrong error is reported.

Below is a working example. The error should be reporting URL, but it is reporting quantity instead. It apparently reports the first field no matter what is is.

const { graphql, buildSchema } = require("graphql");
const { mergeSchemas } = require("graphql-tools");

const rootValue = {
  items: [
    { quantity: 1, URL: "Test1" },
    { quantity: 1, URL: "Test2" },
    { quantity: 3, URL: "Test3" },
    { quantity: 3 },
    { quantity: 1, URL: "Test5" }
  ]
};

const schema1 = buildSchema(`
  type Data {
     items: [LineItem]!
  }

  type LineItem {
      quantity: Int!
      URL: String!
  }

  schema { query: Data }
`);

const schema = mergeSchemas({ schemas: [schema1] });

graphql({
  schema,
  source: "{ items { quantity URL } }",
  rootValue
}).then(result => console.log(JSON.stringify(result, null, 2)));

We use mergeSchemas a lot and this error is pretty critical as it causes developers to spend debugging time in the wrong area of the code base.

@yaacovCR
Copy link
Collaborator

yaacovCR commented Apr 1, 2020

Should be fixed by #1307, rolling into #1306

@yaacovCR yaacovCR closed this as completed Apr 1, 2020
@yaacovCR yaacovCR mentioned this issue Apr 1, 2020
22 tasks
@Grmiade
Copy link

Grmiade commented Jun 15, 2020

@yaacovCR It seems this bug is not fixed when I run the @mikebm's example with the new stitchSchemas function (v6.0.9), I have this error:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field LineItem.quantity.",
      "locations": [
        {
          "line": 1,
          "column": 11
        }
      ],
      "path": [
        "items",
        3,
        "quantity"
      ]
    }
  ],
  "data": {
    "items": [
      {
        "quantity": 1,
        "URL": "Test1"
      },
      {
        "quantity": 1,
        "URL": "Test2"
      },
      {
        "quantity": 3,
        "URL": "Test3"
      },
      null,
      {
        "quantity": 1,
        "URL": "Test5"
      }
    ]
  }
}

@yaacovCR yaacovCR reopened this Jun 15, 2020
yaacovCR added a commit that referenced this issue Jun 16, 2020
yaacovCR added a commit that referenced this issue Jun 17, 2020
yaacovCR added a commit that referenced this issue Jun 19, 2020
fix includes changes to error proxying
= no longer relying on graphql-js to automatically set path of stitched error
= now relying on graphql-js not modifying path of resolver-returned error even if references a path different from resolver
= streamlines object error annotations to represent a map of errors by the field name
= introduces depth property for proxied results that must be updated during proxied result traversal for properly searching the error arrays
= requires updating of advanced wrapping/hoisting resolvers
@yaacovCR yaacovCR added the bug label Jul 12, 2020
yaacovCR added a commit that referenced this issue Aug 3, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 23, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 23, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 23, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
@yaacovCR yaacovCR mentioned this issue Aug 23, 2020
Merged
yaacovCR added a commit that referenced this issue Aug 24, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 25, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 31, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Aug 31, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 2, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 4, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 13, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 21, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 21, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 21, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 21, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Sep 30, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Oct 1, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Oct 6, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
yaacovCR added a commit that referenced this issue Oct 23, 2020
This is not the optimal way to address #1047 but is the most backwards
compatible.
@yaacovCR
Copy link
Collaborator

yaacovCR commented Nov 2, 2020

Fixed in v7

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

No branches or pull requests

3 participants