add clarity to ID lifetime, mutual "complete" and unknown IDs

[kristjanvalur]

The protocol specification specifies what to do when a subscribe message arrives with an id corresponding to an active operation. However, it doesn't say whether it needs to keep track of already completed operations.

Is it permissible to re-use an ID on a connection if it has been previously completed by the server?

Similarly, it doesn't specify what should happen when a next or complete message is seen with an unknown (or already completed) ID.

This is important because of the inherently racy nature of the complete message. Both client and server can issue a complete message when the other party has already done the same, and the message is in flight.

• when a client issues a complete, an error, next or a complete message may be underway from the server.
• when the server issues a complete, the client may have already issued a complete and it is somewhere in-flight.

In my opinion the protocol spec should be augmented with the following:

1. Specify that operation IDs need only be kept track of while active and that a client is free to re-use an ID that the server has previously completed (with an error or complete message.
2. Explain the asyncronous nature of client-to-server complete messages and how sending a complete message cannot guarantee that a message (error, next, complete) can yet be received after sending it.
3. Specify that receiving a message pertaining to a completed or unknown ID should be discarded, or only cause a disconnect if it can be determined that such an id has never been seen on the connection.

Furthermore, single-result-operations can be more easily explained protocol-wise of being just special cases of subscriptions, where there will be one, and only one, next message (unless it is preceded by an error message`.

The reason I am suggesting this is that I am in the process of writing a client implementation against a server implementation which violates 3. above, i.e. it will cause a disconnect when two complete messages race. Having specific guidelines in the protocol would help implementors to avoid such mistakes.

Any thoughts?

[enisdenjo]

Hey hey, I am sorry for the late replay, haven't had time to read into this with care.

Is it permissible to re-use an ID on a connection if it has been previously completed by the server?

The Protocol makes no assumptions in this regard, so yes.

Thanks for the elaborate explanation and great thinking, I agree with you - let's see if we're in line over at #342.

[kristjanvalur]

After you merged #342, and working on my client, It occurred to me that Error messages as an alternative to Next and Complete are a bit ambiguous.

The graphql ExecutionResult, when errors is non-empty, can contain a data member containing none, indicating that the error occurred during evaluation, and not as an initial validation.

the Error message contains no such member. So, receiving an Error message (having not previously received a Next) there is no way to know if the original Subscribe message wasn't successful, or if the Error happened when preparing the first result.

In light of this, I think it should be strongly advised to use the Next message only as an error response to the Subscribe message and handle all other execution errors via a Next message.

[enisdenjo]

The Error message must not necessarily contain a GraphQL related error, it can be any error fatal for the subscription.

In light of this, I think it should be strongly advised to use the Next message only as an error response to the Subscribe message and handle all other execution errors via a Next message.

I'll assume you meant advising to use the Error message as the error response (not the Next message). TBH, I don't want to advise anything, I want the people to make their own assumptions and decisions only given the constraints of the Protocol.

Sending an Error message at any given time in the subscription is not illegal and must be handled appropriately, advising not to use it leaves room for implementation oversights (like not handling it after the first Next message).

[kristjanvalur]

Well, on the other hand, it can result in different server implementations adopting different policies, making it harder to write client libraries that work with both. The error.payload is currently specified to contain an array of GraphQLError, exactly like the errors member of ExecutionResult. Protocols are often very specific about what is allowed, what isn't, and what is undefined, precisely to ensure interoperability.

Reading the spec originally left me wondering why an error message was needed at all, since a next can already do the same thing, and the protocol is already pretty minimal (there is no subscribe-ack message, for instance). So I originally assumed that there had to be some semantic difference between an error. and a next-with-error+complete.

As I explained, next can already semantically distinguish between an error during pre-execution and an error during execution (the data=null member). next cannot do that, so receiving that message won't tell you which it is.

Anyway, this is probably a fine point and not of consequence. In my current application, I am treating both as the same and not really trying to distinguish between the two types of errors (pre-exeuction and during-execution).

Cheers!

[enisdenjo]

GraphQL over WebSocket is a transport protocol, not a GraphQL protocol. How you handle the semantics of GQL is utterly irrelevant to the transport protocol.

The meaning behind the presence of the error field in the GraphQL execution result is defined by the GraphQL Protocol; in plain words - GraphQL over WebSocket doesn't (and shouldn't) care - an implementor might as well ignore it completely, the transport will still work.

The Error message, on the other hand, is a transport message relevant to the subscription status. The message payload being an array of GraphQLErrors is only to bring consistency with the GQL format, which is an extension of the native Error + some helpful GQL debugging fields.

[kristjanvalur]

Yes, I get it. Been reading a bit in the graphql spec as well today. I suppose it makes sense to view the error message as a graphql-transport-ws message, including validation errors and unexpected stream termination and so on, whereas the regular "execution result" is what comes out of the graphql machinery itself.

All good, then. thanks for your explanation.

[roksui]

Thank you for this clarification to the protocol. However, I still have a question about whether the protocol places any constraints on how to handle complete messages (and possibly other types that also require the id to process the request) that have no id field.

Does a complete message without the id field count as an "invalid message" described here, and therefore be considered as an error and the connection should be closed? Or is it considered an unknown ID that should be ignored without closing the connection?

At the start of the protocol document, it says that the id field is optional, in which case I don't think having a missing id field should be considered an error, but I would like to know if there are any actions to take by the receiver. (For example, just ignoring the message)

[enisdenjo]

Does a complete message without the id field count as an "invalid message" described here, and therefore be considered as an error and the connection should be closed? Or is it considered an unknown ID that should be ignored without closing the connection?

Any message not conforming exactly to the formats described in the protocol are considered invalid and will have the connection terminated.

At the start of the protocol document, it says that the id field is optional, in which case I don't think having a missing id field should be considered an error, but I would like to know if there are any actions to take by the receiver. (For example, just ignoring the message)

Ah yes, I said that because the id is not present in the ConnectionInitMessage, ConnectionAckMessage, PingMessage and PongMessage. But is required as per the type definitions in every other message type.

Thanks, that's a great spot! I would be happy to accept a PR clarifying this!