router
router copied to clipboard
Remove `@` from error paths
When a subgraph returns an unexpected response (ie not a body with at least one of errors or data), the errors surfaced by the router include an @ in the path which indicates an error applied to all elements in the array. This is not a behavior defined in the GraphQL spec and is not easily parsed.
This PR expands the @ symbol to reflect all paths that the error should apply to.
The tests reflect the following federated graph with 4 subgraphs:
products ----> inventory
\---> reviews
\---> accounts
## router response (all subgraph calls successful)
{
"data": {
"topProducts": [
{
"name": "Table",
"inStock": true,
"reviews": [
{"id": "1", "author": {"username": "@ada", "name": "Ada"}},
{"id": "1", "author": {"username": "@alan", "name": "Alan"}}
]
},
{
"name": "Chair",
"inStock": false,
"reviews": [
{"id": "3", "author": {"username": "@alan", "name": "Alan"}}
]
}
]
}
}
Currently, if the inventory subgraph returns a malformed response, the router response will look like:
{
"data": {"topProducts": [{"name": "Table", "inStock": Null, "reviews": [{"id": "1", "author": {"username": "@ada", "name": "Ada"}}, {"id": "1", "author": {"username": "@alan", "name": "Alan"}}]}, {"name": "Chair", "inStock": Null, "reviews": [{"id": "3", "author": {"username": "@alan", "name": "Alan"}}]}]},
"errors": [
{
"message": "service 'inventory' response was malformed: graphql response without data must contain at least one error",
"path": ["topProducts", "@"],
"extensions": {"service": "inventory", "reason": "graphql response without data must contain at least one error", "code": "SUBREQUEST_MALFORMED_RESPONSE"}
}
]
}
With this change, the response will look like:
{
"data":{"topProducts":[{"name":"Table","inStock":null,"reviews":[{"id":"1","author":{"username":"@ada","name":"Ada"}},{"id":"1","author":{"username":"@alan","name":"Alan"}}]},{"name":"Chair","inStock":null,"reviews":[{"id":"3","author":{"username":"@alan","name":"Alan"}}]}]},
"errors": [
{
"message": "service 'inventory' response was malformed: graphql response without data must contain at least one error",
"path":["topProducts",0],
"extensions":{"service":"inventory","reason":"graphql response without data must contain at least one error","code":"SUBREQUEST_MALFORMED_RESPONSE"}
},
{
"message":"service 'inventory' response was malformed: graphql response without data must contain at least one error",
"path":["topProducts",1],
"extensions":{"service":"inventory","reason":"graphql response without data must contain at least one error","code":"SUBREQUEST_MALFORMED_RESPONSE"}
}
]
}
TODO:
- [ ] Would it be better to just specify
0as the element rather than expanding all the paths? - [ ] Is
inverted_pathsthe best way to handle this? - [ ] Should this be applied in the
Fetchstep of the query plan, or should it take place in theFlattenstep? I put it in theFetchstep because that's where theinverted_pathsvariable is accessible, so I think that's where we know all the paths the error applies to. - [ ] Document the reasoning behind
interim_errorsinline - [ ] Use snapshots to capture which errors are actually present rather than just counting the number of errors (in the tests)
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
- [ ] Changes are compatible[^1]
- [ ] Documentation[^2] completed
- [ ] Performance impact assessed and acceptable
- [ ] Metrics and logs are added[^3] and documented
- Tests added and passing[^4]
- [ ] Unit Tests
- [ ] Integration Tests
- [ ] Manual Tests
Exceptions
Note any exceptions here
Notes
[^1]: It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this.
[^2]: Configuration is an important part of many changes. Where applicable please try to document configuration examples.
[^3]: A lot of (if not most) features benefit from built-in observability and debug-level logs. Please read this guidance on metrics best-practices.
[^4]: Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions.