move the nullifying error messages to extensions#2077
Conversation
Adding them in the list of errors was potentially redundant (subgraph can already add an error message indicating why a field is null) and could be treated as a failure by clients, while nullifying fields is a part of normal operation. We still add the messages in extensions so clients can easily debug why parts of the response were removed
This comment has been minimized.
This comment has been minimized.
|
now accepting naming suggestions for the new extensions field. I put "formatting", but maybe that could be "nullability"? WDYT? |
|
I prefer |
garypen
left a comment
garypen
left a comment
There was a problem hiding this comment.
a few questions/comments about the general design. The code looks good.
apollo-router/src/spec/query.rs
Outdated
| response.errors.extend(parameters.errors.into_iter()); | ||
| if !parameters.errors.is_empty() { | ||
| response.extensions.insert( | ||
| "formatting", |
There was a problem hiding this comment.
Is "formatting" the correct name? Would "composition" be a better choice or "assembly". Is there an established convention for this kind of naming?
UPDATE: I just saw the other comments about naming. I don't think "nullability" is a good name, but could be convinced.
There was a problem hiding this comment.
Possibly value-completion? As this is the part of the spec that handles nullability https://spec.graphql.org/June2018/#sec-Value-Completion
|
|
||
| let request = supergraph::Request::fake_builder() | ||
| .query("query { currentUser { activeOrganization { id creatorUser { name } } } }") | ||
| // Request building here |
There was a problem hiding this comment.
You can probably remove this comment.
6 participants
Fix #2071
Adding them in the list of errors was potentially redundant (subgraph can already add an error message indicating why a field is null) and could be treated as a failure by clients, while nullifying fields is a part of normal operation. We still add the messages in extensions so clients can easily debug why parts of the response were removed