-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarification of how to handle errors that occur in a subscription event stream #995
Comments
Hi there. I want to +1 this issue, as we're running into a difficult challenge here. We're using (Notice in this example how this isn't an error creating the source event stream. The source event stream is successfully created, and a first source event is even
As @Yogu has noted above, this is obviously really bad for us. This interrupts every other subscription the client may be subscribed to at that moment, adds reconnection overhead, drops events, etc. And if we're experiencing some downtime on a specific subscription/source stream, this'll result in repeat disconnect-reconnect thrash, because the client also has no signal on which subscription has failed!! — We can change our code to:
And translates them into a Doing this would obviously be pretty manual, though, and we'd have to do it for every subscription we have. — As noted in @Yogu's comment above, the GraphQL spec is explicit about handling errors when resolving source events to response events (as
So it's only iterating over source event streams — the "middle" step of the algorithm — where I'm unclear if this is intentional in the spec or not, but it doesn't feel like good/desirable behavior to me. Would it be possible to clarify this in the spec, and ideally, gracefully handle and return Thank you for the consideration! |
For this issue to get more attention, I suggest you bring it to a GraphQL Working Group; the next one is on Thursday: https://github.com/graphql/graphql-wg/blob/main/agendas/2024/03-Mar/07-wg-primary.md |
The GraphQL specification is pretty explicit in regards to handling of errors that occur within a field resolver. It does not say a lot about errors that occur while generating the events of a subscription field. Is this an intentional omission with the intention that implementations come up with their own strategies for error handling, or should this be specified in more detail?
I asked this question two days ago in the Discord server in #graphql-spec, and @benjie suggested to create an issue, so here it is.
Event stream errors in the spec
Section 6.2.3 defines "event streams" and mentions the possibility of errors:
I did not find any other reference to errors in regards to event streams or their usage in subscriptions. This makes me wonder why the specification mentions these two cases (normal completion vs. completion due ton an error). It could be interpreted as "if an error occurs, the event stream should be completed", or it could mean "if an error occurs, the event stream may be completed", or it could just be an non-normative hint.
Section 5.2.3.2 defines the operation defines an operation to convert a source stream to a response stream. (by the way, why is it called
MapSourceToResponseEvent
and notMapSourceStreamToResponseStream
?) This operation does not mention errors, but it defines that the response stream should be completed when the source stream completes. Section 5.2.3 definesSubscribe
which usesMapSourceToResponseEvent
, but just returns it. This then bubbles up to the introduction of section 6 where it states that the result should be "formatted according to the Response section below". Section 7 (the "below") does not mention streams or subscriptions, so it does not further define how a completion would be formatted.Field resolver errors in the spec
In contrast, for errors that occur in resolvers, there is a whole subsection in the specification: 6.4.4 "Handling Field Errors". It defines where an error can occur, it defines what should happen, and 7.1.2 Errors defines how the errors should be serialized.
Comparing field resolver errors and event stream errors
Field resolvers and field streams are very similar in the specification.
ResolveFieldValue
"calls" the "internal function provided by objectType for determining the resolved value of a field named fieldName".ResolveFieldEventStream
"calls" the "internal function provided by subscriptionType for determining the resolved event stream of a subscription field named fieldName". Both are intended to be implemented by users of the graphql implementation. This is application code, so so they can produce all kinds of unforseen errors.The "internal function provided by subscriptionType for determining the resolved event stream of a subscription field" could produce errors in two ways:
a. The function itself could raise an error
b. The generated "resolved event stream" could experience an error while it is generating events.
I think both of these error scenarios are comparable to an error thrown by the "internal function provided by objectType for determining the resolved value of a field". However, only errors of field resolvers are "caught" in the specification (please correct me if I'm wrong). Especially handling of errors generated like described in b. is not described.
What implementations and graphql servers can do
Errors raised while calling the "internal function provided by subscriptionType for determining the resolved event stream of a subscription field" could be handled like describe din 6.4.4 Handling Field Errors. I think this is pretty reasonable, and I guess there are implementations that already do this. (Maybe it's already specced like this and I just don't understand it)
Errors that occur during the execution of an event stream are a bit more complicated.
Summary
I think it would be great if the specification specified error handling. Alternatively, I it could also clarify that it is up to implementations and wire protocols to provide this. Currently, these might omit error handling and say "working as specified, we can't do anything".
The text was updated successfully, but these errors were encountered: