Issue #13465 - ensure that WebSocketClient onHandshakeResponse is not called twice #13486
Conversation
… called twice Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
...e-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java
Show resolved
Hide resolved
sbordet
left a comment
sbordet
left a comment
There was a problem hiding this comment.
@lachlan-roberts yes it should also fix #13466.
The fix should be less flaky, so the test failures should be investigated.
|
@sbordet I have investigated the failures. In the test the client is trying to upgrade to websocket over HTTP/2 and the server is throwing an exception in the websocket creator. When the client sends the headers frame to upgrade it does not include an END_STREAM flag because the subsequent data frames it sends will be websocket bytes. The server sees the exception and tries to send a 500 response, it sends a HEADERS frame with the END_STREAM flag. The client then has a race whether it processes the HEADERS frame with the 500 response, or the RST_STREAM frame first. |
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
|
@lachlan-roberts I fixed the tests, please review the fix. The build failed due to #13533, so I think this PR is fine now. |
…13465-WebSocketClientResponseListener
|
@sbordet it seems after merging with |
Now testing whether the request was a tunnel, and if so, try to close the request side of the stream. If the close is successful, it means a response was already sent, so we can avoid sending a RST_STREAM. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
… tunnel. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
github-project-automation
bot
moved this from 👀 In review
to ✅ Done
in Jetty 12.1.1 - FROZEN
lachlan-roberts
deleted the
jetty-12.1.x-13465-WebSocketClientResponseListener
branch
This PR is an addendum to #13486, that improves the handling of WebSocket upgrade failures. If there was a failure in the HTTP upgrade, it is now retained and handled as-is, not converted to an UpgradeException. This should ensure better backward compatibility with 12.0.x. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
* Fixes #13466 - Lost WebSocket upgrade failure. This PR is an addendum to #13486, that improves the handling of WebSocket upgrade failures. If there was a failure in the HTTP upgrade, it is now retained and handled as-is, not converted to an UpgradeException. This should ensure better backward compatibility with 12.0.x. Signed-off-by: Simone Bordet <simone.bordet@gmail.com> * Fixed test expectations. Signed-off-by: Simone Bordet <simone.bordet@gmail.com> --------- Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
closes #13465