Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

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

No branches or pull requests

3 participants