Handling close connection with GraphQLWSHandler

[aleksandar78]

Describe the feature

Add endHandler(Handler endHandler) to GraphQLWSHandler.

Use cases

Until vert.x 4.* graphql subscription APIs have been protected (in my case) with the help of 3 methods of ApolloWSHandler:

• connectionInitHandler (check if the payload contains a valid token and pair into session map user info with server web socket text handler id)
• queryContext (inject user info from session map into request by given text handler id)
• endHandler (remove user info paired with text handler id)
  This approach worked with ApolloWSHandler.

The handler above is deprecated. The suggestion is to use GraphQLWSHandler.
The main problem is that the GraphqlWSHandler has only connectionInitHandler and beforeExecute (substitution for queryContext). What is missing is endHandler.
I don't know if this was intentional or not but the common authentication flow with open connections has the problem of removing user info when the connection is closed. The end handler is very handy to keep the session map clean over time.

Implementation

Watching the source code of ApolloWSConnectionHandler (ApolloWSHandler) and ConnectionHandler (GraphQLWSHandler) I found the difference in close handler methods where:

ApolloWSConnectionHandler

private void close(Void v) {
    subscriptions.values().forEach(Subscription::cancel);

    Handler<ServerWebSocket> eh = apolloWSHandler.getEndHandler();
    if (eh != null) {
      eh.handle(serverWebSocket);
    }
  }

ConnectionHandler

public void close() {
      subscriptions.values().forEach(Subscription::cancel);
}

ConnectionHandler doesn't have any reference to endHandler. Maybe, by adding the reference the same behavior could be obtained.

I admit that this analysis is very superficial but it could be a starting point.

[tsegismont]

@aleksandar78 looks like an omission to me. Would you like to contribute the enhancement?

[aleksandar78]

@tsegismont I found a way to pass the handler when the close socket event is called.
Inside connectionInitHandler where init Message is passed as the argument for the first time, the WebSocket instance can be used for the same purpose as endHandler in the ApolloWSHandler.

This is the trick that works well but it's not clear on the GraphQLWSHandler API level, IMO. The solution could be exposing endHandler or adding an example in official documentation to explain how to handle this kind of situation.

What do you think?

Anyway, I'm available to help.

[tsegismont]

Can you paste a snippet to better understand your workaround?

[aleksandar78]

This is kotlin pseudo-code passed to GraphQLWSHandler.connectionInitHandler

fun onInit(event: ConnectionInitEvent) {
    val socket = event.message().socket()

    // validate token sent by graphql client
    validateToken(event.message())
      .compose { username ->
          fetchUser(username)
      }
      .onSuccess { user -> 
        val textHandlerID = socket.textHandlerID()
        sessionMap[textHandlerID] = user
        socket.closeHandler {
          sessionMap.remove(textHandlerID)
        }
        event.complete()
      }
      .onFailure { ex ->
        val error = createAuthException(ex, socket)
        event.fail(error)
      }
  }

[tsegismont]

This is kotlin pseudo-code passed to GraphQLWSHandler.connectionInitHandler

Thank you @aleksandar78

The problem with this code is that the WebSocket can have only one closeHandler, which means this will not be invoked:

vertx-web/vertx-web-graphql/src/main/java/io/vertx/ext/web/handler/graphql/impl/ws/ConnectionHandler.java

Lines 117 to 119 in 296064d

	private void close(Void unused) {
	state.close();
	}

So, having an endHandler to set in io.vertx.ext.web.handler.graphql.ws.GraphQLWSHandler and invoking it in io.vertx.ext.web.handler.graphql.impl.ws.ConnectionHandler#close seems like the right thing to do

[aleksandar78]

@tsegismont can you point to me vert.x contributor's guide (if any) before I make the pull request?
I can make the effort but I'm not familiar with your branching strategy.

Thanks

[tsegismont]

https://github.com/eclipse-vertx/vert.x/blob/master/CONTRIBUTING.md

First, you need to make a pull request to master branch (which is where Vert.x 5 development happens now). Then you can backport to the 4.x branch (by creating another pull request).

[tsegismont]

Closed by fe47bdf