Add errors property to @connect and @source#3250
Conversation
|
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
dylan-apollo
left a comment
dylan-apollo
left a comment
There was a problem hiding this comment.
Make sure you also have an open task to update the LSP definitions of this and to write some docs about it!
| ConnectBatch.addField(new InputFieldDefinition('maxSize')).type = schema.intType(); | ||
| connect.addArgument('batch', ConnectBatch); | ||
|
|
||
| const ConnectErrors = schema.addType(new InputObjectType(this.typeNameInSchema(schema, CONNECT_ERRORS)!)); |
There was a problem hiding this comment.
Should we make this and the source one a single ConnectorsErrors type for simplicity since they're identical? Like how we share headers.
There was a problem hiding this comment.
🤔 I think the definitions should be the same. The validation in Rust is slightly different (implementation is different and variables available are different) but yea, the actual fields should be identical.
I'd have to change this in Rust too so I'll wait till you're done reviewing to actually execute this change to make sure we're still confident in it once you're done looking at both PRs 😅
| ConnectErrors.addField(new InputFieldDefinition('message')).type = JSONSelection; | ||
| ConnectErrors.addField(new InputFieldDefinition('extensions')).type = JSONSelection; |
There was a problem hiding this comment.
If I understand the design correctly, setting either message or extensions overrides the default behavior for both, right?
I would suggest that either we retain the default behavior when one is unset or we make these both required fields (here and in our better validations) so it's very clear to the user that they're turning off the default message (and getting an empty string??) if they set custom extensions.
There was a problem hiding this comment.
From the design:
Configuring @source(error:) or @connect(error:) overrides default or fallback values for both message and extensions, even if extensions is not set.
I interpret that as saying that that both fields are optional... but yea, I agree that it's a bit of a confusing behaviour to have one of the fields just "disappear" if not set.
So, my rust implementation allows you to override just message or extensions (or both obviously). (see: https://github.com/apollographql/router/pull/7311/files#diff-1c93268ec072033d27937077ca358823a0569f2ac1919d2976059c7a625f1fd6)
3 participants
Add errors property to @connect and @source