@defer: error related to a field in a deferred fragment appears in chunk for non-deferred fragment #1818

BoD · 2022-09-16T13:34:02Z

Describe the bug
Using the graphql test suite as an inspiration, I am selecting a field that resolves to an error, in a deferred fragment:

query HandlesErrorsThrownInDeferredFragmentsQuery {
  computer(id: "Computer1") {
    id
    ...ComputerErrorField @defer
  }
}

fragment ComputerErrorField on Computer {
  errorField
}

I am receiving the error in the first chunk (along with id) and no errors in the second chunk, whereas I would have expected the opposite (error in the 2nd chunk along errorField), as graphql-js apparently does.

Unsure if this is the spec really mandates this, it may be just a different and acceptable implementation, if so feel free to close!

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"errors":[{"message":"Subgraph errors redacted"}],"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":{"errorField":null},"path":["computer"]}]}
--graphql--

To Reproduce

git clone [email protected]:apollographql/apollo-kotlin.git
cd apollo-kotlin && git checkout defer-with-router-tests
(cd tests/defer/router/subgraphs/computers && npm install && APOLLO_PORT=4001 npm start)&
path/to/router --supergraph tests/defer/router/simple-supergraph.graphqls &
curl --request POST     --header 'content-type: application/json'     --url http://localhost:4000     --data '{"query":"query HandlesErrorsThrownInDeferredFragmentsQuery {  computer(id: \"Computer1\") {    id    ...ComputerErrorField @defer  } }  fragment ComputerErrorField on Computer {  errorField }"}' --header 'Accept: multipart/mixed; deferSpec=20220824'

The text was updated successfully, but these errors were encountered:

BoD · 2022-09-16T15:21:41Z

It's a bit worse if the field is not nullable: because of bubbling up we never get the id field

actual:

--graphql
content-type: application/json

{"data":{"computer":null},"errors":[{"message":"Subgraph errors redacted"}],"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":null,"path":["computer"]}]}
--graphql--

whereas in theory we should be able to get it in the first chunk.

expected:

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":null,"errors":[{"message":"Subgraph errors redacted"}],"path":["computer"]}]}
--graphql--

Geal · 2022-09-19T11:04:56Z

Could you add this to the config so we see xhat error is really sent?:

include_subgraph_errors:
  all: true

Are all fields returned by a single subgraph request? That might be the reason here: the primary response aggregates errors from its subgraph calls and returns it, then deferred responses do the same from their own subgraph calls, but if they did not call a subgraph and instead return data from the primary's subgraph call, the error is probably not returned.

If that's the case, I think that could be fixed by transmitting to the deferred fragment execution the errors previously generated, then when creating the response, we would filter which errors are supposed to be returned

BoD · 2022-09-19T12:01:38Z

Yes, in my test I actually have only one subgraph.

Here's the result with the full errors:

Nullable field:

{
  "data": {
    "computer": {
      "id": "Computer1"
    }
  },
  "errors": [
    {
      "message": "Error field",
      "locations": [
        {
          "line": 1,
          "column": 93
        }
      ],
      "path": [
        "computer",
        "errorField"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "Error: Error field",
            "    at Object.errorField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/computers.js:23:19)",
            "    at field.resolve (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/apollo-server-core/dist/utils/schemaInstrumentation.js:56:26)",
            "    at executeField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:481:20)",
            "    at executeFields (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)",
            "    at completeObjectValue (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:914:10)",
            "    at completeValue (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:635:12)",
            "    at executeField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:489:19)",
            "    at executeFields (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)",
            "    at executeOperation (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:344:14)",
            "    at execute (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:136:20)"
          ]
        }
      }
    }
  ]
}

Non nullable field:

{
  "data": {
    "computer": null
  },
  "errors": [
    {
      "message": "Error field",
      "locations": [
        {
          "line": 1,
          "column": 93
        }
      ],
      "path": [
        "computer",
        "nonNullErrorField"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "Error: Error field",
            "    at Object.nonNullErrorField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/computers.js:26:19)",
            "    at field.resolve (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/apollo-server-core/dist/utils/schemaInstrumentation.js:56:26)",
            "    at executeField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:481:20)",
            "    at executeFields (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)",
            "    at completeObjectValue (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:914:10)",
            "    at completeValue (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:635:12)",
            "    at executeField (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:489:19)",
            "    at executeFields (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)",
            "    at executeOperation (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:344:14)",
            "    at execute (/Users/bod/gitrepo/apollo-kotlin-1/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:136:20)"
          ]
        }
      }
    }
  ]
}

BoD · 2022-11-02T17:27:36Z

👋 I'm still seeing the same issue with v1.2.1.

To reproduce:

git clone [email protected]:apollographql/apollo-kotlin.git
cd apollo-kotlin
(cd tests/defer/router/subgraphs/computers && npm install && APOLLO_PORT=4001 npm start)&
path/to/router --supergraph tests/defer/router/simple-supergraph.graphqls &
curl --request POST     --header 'content-type: application/json'  \
    --url http://localhost:4000/   \
    --data '{"query":"query HandlesErrorsThrownInDeferredFragmentsQuery {  computer(id: \"Computer1\") {    id    ...ComputerErrorField @defer  } }  fragment ComputerErrorField on Computer {  errorField }"}' --header 'Accept: multipart/mixed; deferSpec=20220824'

query:

query HandlesErrorsThrownInDeferredFragmentsQuery { 
  computer(id: "Computer1") {   
    id
    ...ComputerErrorField @defer
  }
}
fragment ComputerErrorField on Computer {
  errorField
}

Expected:

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":{"errorField":null},"errors":[{"message":"Subgraph errors redacted"}],"path":["computer"]}]}
--graphql--

Actual:

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"errors":[{"message":"Subgraph errors redacted"}],"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":{"errorField":null},"path":["computer"]}]}
--graphql--

Fix #1818 Fix #2185 Blocked by #2109 (waiting for the router-bridge update) When errors are generated during the primary execution, some of them can be affected to deferred responses. To handle that, we need to: - transmit errors from the primary query to deferred node execution along with the primary fetches - be able to check if an error path belongs to a deferred query Since the error path may belong to a part of the response that was nullified, we need to follow the error path through the primary or deferred queries Co-authored-by: Jeremy Lempereur <[email protected]>

BoD · 2022-12-15T11:55:37Z

I can still reproduce on 1.6.0 with the same steps as above

Geal · 2022-12-15T12:03:41Z

thank you for testing it, I'll check

Geal · 2022-12-15T12:08:22Z

I just tested on the branch update-defer-test-with-router-1-6-0

And I get this:

$ curl --request POST     --header 'content-type: application/json'     --url http://localhost:4000     --data '{"query":"query HandlesErrorsThrownInDeferredFragmentsQuery {  computer(id: \"Computer1\
") {    id    ...ComputerErrorField @defer  } }  fragment ComputerErrorField on Computer {  errorField }"}' --header 'Accept: multipart/mixed; deferSpec=20220824'
                                                           
--graphql                                                                                                                                                                                                                                     
content-type: application/json                                                                                                                                                                                                                
                                                                                                                                                                                                                                              
{"data":{"computer":{"id":"Computer1"}},"hasNext":true}                                                                                                                                                                                       
--graphql                                                                                                                                                                                                                                     
content-type: application/json                                                                                                                                                                                                                
                                                                                                                                                                                                                                              
{"hasNext":false,"incremental":[{"data":{"errorField":null},"path":["computer"],"errors":[{"message":"Error field","locations":[{"line":1,"column":93}],"path":["computer","errorField"],"extensions":{"code":"INTERNAL_SERVER_ERROR","excepti
on":{"stacktrace":["Error: Error field","    at Object.errorField (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/computers.js:28:19)","    at field.resolve (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/c
omputers/node_modules/apollo-server-core/dist/utils/schemaInstrumentation.js:56:26)","    at executeField (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:481:20)","    at exe$
uteFields (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)","    at completeObjectValue (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_module
s/graphql/execution/execute.js:914:10)","    at completeValue (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:635:12)","    at executeField (/home/geal/dev/apollo-kotlin/tests
/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:489:19)","    at executeFields (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)","    at exe
cuteOperation (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:344:14)","    at execute (/home/geal/dev/apollo-kotlin/tests/defer/router/subgraphs/computers/node_modules/graphq
l/execution/execute.js:136:20)"]}}}]}]} 
--graphql--

so this looks like the expected result? Are you sure this is using the 1.6 router?

BoD · 2022-12-15T13:08:34Z

Odd! It does says 1.6.0 when starting it.

Wait the behavior is different with --dev! 🤔

Without:

$ ~/Tmp/router/router --supergraph tests/defer/router/simple-supergraph.graphqls
2022-12-15T13:06:51.075503Z  INFO Apollo Router v1.6.0 // (c) Apollo Graph, Inc. // Licensed as ELv2 (https://go.apollo.dev/elv2)
2022-12-15T13:06:51.075552Z  INFO Anonymous usage data is gathered to inform Apollo product development.  See https://go.apollo.dev/o/privacy for more info.
2022-12-15T13:06:51.416303Z  INFO healthcheck endpoint exposed at http://127.0.0.1:8088/health
2022-12-15T13:06:51.416603Z  INFO GraphQL endpoint exposed at http://127.0.0.1:4000/ 🚀

$ curl --request POST     --header 'content-type: application/json'      --url http://localhost:4000/       --data '{"query":"query HandlesErrorsThrownInDeferredFragmentsQuery {  computer(id: \"Computer1\") {    id    ...ComputerErrorField @defer  } }  fragment ComputerErrorField on Computer {  errorField }"}' --header 'Accept: multipart/mixed; deferSpec=20220824'

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"errors":[{"message":"Subgraph errors redacted"}],"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":{"errorField":null},"path":["computer"]}]}
--graphql--

With:

$ ~/Tmp/router/router --dev --supergraph tests/defer/router/simple-supergraph.graphqls
2022-12-15T13:04:19.814946Z  INFO Running with *development* mode settings which facilitate development experience (e.g., introspection enabled)
2022-12-15T13:04:19.815016Z  INFO Apollo Router v1.6.0 // (c) Apollo Graph, Inc. // Licensed as ELv2 (https://go.apollo.dev/elv2)
2022-12-15T13:04:19.815022Z  INFO Anonymous usage data is gathered to inform Apollo product development.  See https://go.apollo.dev/o/privacy for more info.
2022-12-15T13:04:20.156597Z  INFO healthcheck endpoint exposed at http://127.0.0.1:8088/health
2022-12-15T13:04:20.156905Z  INFO GraphQL endpoint exposed at http://127.0.0.1:4000/ 🚀

$ curl --request POST     --header 'content-type: application/json'      --url http://localhost:4000/       --data '{"query":"query HandlesErrorsThrownInDeferredFragmentsQuery {  computer(id: \"Computer1\") {    id    ...ComputerErrorField @defer  } }  fragment ComputerErrorField on Computer {  errorField }"}' --header 'Accept: multipart/mixed; deferSpec=20220824'

--graphql
content-type: application/json

{"data":{"computer":{"id":"Computer1"}},"hasNext":true}
--graphql
content-type: application/json

{"hasNext":false,"incremental":[{"data":{"errorField":null},"path":["computer"],"errors":[{"message":"Error field","locations":[{"line":1,"column":93}],"path":["computer","errorField"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Error field","    at Object.errorField (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/computers.js:28:19)","    at field.resolve (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/apollo-server-core/dist/utils/schemaInstrumentation.js:56:26)","    at executeField (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:481:20)","    at executeFields (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)","    at completeObjectValue (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:914:10)","    at completeValue (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:635:12)","    at executeField (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:489:19)","    at executeFields (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:413:20)","    at executeOperation (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:344:14)","    at execute (/Users/bod/gitrepo/apollo-kotlin-0/tests/defer/router/subgraphs/computers/node_modules/graphql/execution/execute.js:136:20)"]}}}]}]}
--graphql--

Geal · 2022-12-15T13:11:45Z

I am testing without --dev though 🤔
Could you add the include_subgraph_errors option so we can see the error messages?

Geal · 2022-12-15T13:16:14Z

my guess here is that without include_subgraph_errors, we replace the errors with an object that does not have a path, so we cannot affect it to the deferred response

BoD · 2022-12-15T13:18:28Z

I confirm. with include_subgraph_errors, I get the correct behavior too (same as with --dev).

Geal · 2022-12-15T13:34:07Z

alright, that should be an easy fix

Fix #1818 the path is used to decide whether to assign the error on the primary or deferred response

BoD added raised by user triage labels Sep 16, 2022

Geal self-assigned this Sep 22, 2022

Geal removed the triage label Sep 22, 2022

Geal mentioned this issue Sep 30, 2022

spread errors on incremental responses #1892

Merged

Geal closed this as completed in #1892 Oct 19, 2022

Geal closed this as completed in 4c33935 Oct 19, 2022

abernix mentioned this issue Oct 25, 2022

release: v1.2.1 #1997

Merged

Geal reopened this Nov 2, 2022

Geal mentioned this issue Dec 1, 2022

Dispatch errors from the primary response to deferred responses #2192

Merged

Geal closed this as completed in #2192 Dec 12, 2022

BrynCooke added this to the v1.6.0 milestone Dec 13, 2022

Geal reopened this Dec 15, 2022

Geal mentioned this issue Dec 15, 2022

keep the error path when redacting subgraph errors #2273

Merged

Geal closed this as completed in #2273 Dec 16, 2022

Geal pushed a commit that referenced this issue Dec 16, 2022

keep the error path when redacting subgraph errors (#2273)

0c4a17f

Fix #1818 the path is used to decide whether to assign the error on the primary or deferred response

abernix removed this from the v1.6.0 milestone Dec 22, 2022

abernix added this to the v1.7.0 milestone Dec 22, 2022

abernix mentioned this issue Dec 23, 2022

release: v1.7.0 #2312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@defer: error related to a field in a deferred fragment appears in chunk for non-deferred fragment #1818

@defer: error related to a field in a deferred fragment appears in chunk for non-deferred fragment #1818

BoD commented Sep 16, 2022

BoD commented Sep 16, 2022

Geal commented Sep 19, 2022

BoD commented Sep 19, 2022 •

edited

Loading

BoD commented Nov 2, 2022 •

edited by Geal

Loading

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

Geal commented Dec 15, 2022

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

Geal commented Dec 15, 2022

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

@defer: error related to a field in a deferred fragment appears in chunk for non-deferred fragment #1818

@defer: error related to a field in a deferred fragment appears in chunk for non-deferred fragment #1818

Comments

BoD commented Sep 16, 2022

BoD commented Sep 16, 2022

Geal commented Sep 19, 2022

BoD commented Sep 19, 2022 • edited Loading

BoD commented Nov 2, 2022 • edited by Geal Loading

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

Geal commented Dec 15, 2022

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

Geal commented Dec 15, 2022

BoD commented Dec 15, 2022

Geal commented Dec 15, 2022

BoD commented Sep 19, 2022 •

edited

Loading

BoD commented Nov 2, 2022 •

edited by Geal

Loading