search

eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.26k stars 2.07k forks source link

WebSocket: introduce a reliable way to detect "WebSocket is closed" error #5221

Open mkouba opened 3 months ago

mkouba commented 3 months ago

Describe the feature

Currently, when the connection is closed but the client still attempts to write a message the API returns a failed Future where the cause is represented as a NoStackTraceThrowable with message WebSocket is closed. Consequently, it's not possible to detect this kind of error in a reliable way.

It might be useful to introduce either (A) a specific exception like WebSocketClosedException or (B) any other means, such as an error code (enum, constant, etc.) stored in the throwable/failed future.

Use cases

In Quarkus, we would like to be able to catch this kind of error and react appropriately.

mkouba commented 3 months ago

CC @geoand @cescoffier @vietj

geoand commented 3 months ago

+1

vietj commented 3 months ago

@mkouba I think you can set a close handler on the WebSocket and be aware of when a WebSocket is closed and cannot send message anymore

mkouba commented 3 months ago

@mkouba I think you can set a close handler on the WebSocket and be aware of when a WebSocket is closed and cannot send message anymore

Yes, that's possible and we use it but for a different purpose. In this particular case, we need to react immediately, e.g. when WebSocket#writeTextMessage(String, Handler<AsyncResult<Void>>) is called.

vietj commented 3 months ago

we totally can do that, I was just pointing out that the common design we advocate is that you set a boolean closed that is set to true when the close handler is called and it can be probed

mkouba commented 3 months ago

we totally can do that, I was just pointing out that the common design we advocate is that you set a boolean closed that is set to true when the close handler is called and it can be probed

I get it and as I already mentioned we use this pattern but it does not really help if you need to analyze a failure produced by WebSocket#writeTextMessage() and friends. Currently, we have to test the exception message like this: https://github.com/quarkusio/quarkus/blob/3.11.1/extensions/websockets-next/runtime/src/main/java/io/quarkus/websockets/next/runtime/Endpoints.java#L250-L262. Which is far from being ideal. As you can see, we test the "closed boolean" first in the connection.isClosed() method but then we still need to analyze the exception in order to react appropriately.