Expose subscription ID to subscription data fetchers #999
Conversation
DgsSSESubscriptionHandler was generating the subscription ID returned to the client, but there was no way for the subscription data fetcher to get access to this ID, which is useful for logging and debugging purposes. To rectify that, change DgsSSESubscriptionHandler to use a Flux directly, which allows us to propagate the subscription id in the context. Now that DgsSSESubscriptionHandler is opinionated about which Publisher implementation it uses, a lot of the code was also simplified by avoiding adapting the subscription Publisher to the SseEmitter ourselves; instead the controller method simply returns a Flux directly and lets Spring handle that. Another change is that any error condition occurring before the GraphQL query is executed successfully (e.g., a Publisher is returned) will result in a non-SSE response; that is, the status code of the response will be non-200, and the error message will no longer be written as a single server-sent event.
| publisher.subscribe(subscriber) | ||
| val subscriptionId = UUID.randomUUID().toString() | ||
| return Flux.from(publisher) | ||
| .contextWrite { ctx -> ctx.put("subscriptionId", subscriptionId) } |
There was a problem hiding this comment.
I can move this key to a constant somewhere
|
|
||
| return ResponseEntity.ok(emitter) | ||
| private fun isSubscriptionQuery(query: String): Boolean { |
There was a problem hiding this comment.
I changed the endpoint to explicitly check if the query is a subscription query before attempting to execute it, which seemed like a good idea to me.
| mapper.writeValueAsString( | ||
| SSEDataPayload( | ||
| data = null, | ||
| errors = listOf(Error(t.message)), |
There was a problem hiding this comment.
I think this line was actually a bug. Error here is actually a typealias for java.lang.Error, so we'd end up sending back a full stack trace. I changed it to use com.netflix.graphql.types.subscription.Error, which just has a message field instead, since I think that is what was intended. Maybe the name of the class should be changed so that it doesn't so easily conflict with the kotlin Error typealias?
|
After some discussion, we decided to handle propagating of the subscription ID separately from refactoring the code to leverage Flux. I'll close this PR in favor of #1001, and open a new one to handle the subscription ID. |
DgsSSESubscriptionHandler was generating the subscription ID returned to the client,
but there was no way for the subscription data fetcher to get access to this ID,
which is useful for logging and debugging purposes. To rectify that, change
DgsSSESubscriptionHandler to use a Flux directly, which allows us to propagate
the subscription id in the context.
Now that DgsSSESubscriptionHandler is opinionated about which Publisher implementation
it uses, a lot of the code was also simplified by avoiding adapting the subscription
Publisher to the SseEmitter ourselves; instead the controller method simply returns
a Flux directly and lets Spring handle that. Another change is that any error condition
occurring before the GraphQL query is executed successfully (e.g., a Publisher is returned)
will result in a non-SSE response; that is, the status code of the response will
be non-200, and the error message will no longer be written as a single server-sent event.