Implement deterministic WebSocket close semantics in Http4s client backend#772
Merged
Conversation
Contributor
|
Clue doesn't properly close http4s websocket JDK clients. This is causing issues on JDK >= 21 because of stricter semantics in the JDK. I'll verify whatever Copilot thinks up here tomorrow once it has finished |
Agent-Logs-Url: https://github.com/gemini-hlsw/clue/sessions/923c72ae-86a8-4f72-a48f-25fb12971c4a Co-authored-by: hugo-vrijswijk <10114577+hugo-vrijswijk@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Implement proper WebSocket close semantics
[WIP] Implement deterministic WebSocket close semantics in Http4s backend (requested by @hugo-vrijswijk)
May 7, 2026
There was a problem hiding this comment.
Pull request overview
Implements deterministic teardown for http4s/JDK WebSocket connections so ApolloClient.disconnect() actually releases the underlying high-level WS resource and avoids lingering receive fibers; also adds a focused regression test and updates CI to exercise newer JDKs.
Changes:
- Add guarded, idempotent close/release semantics to
Http4sWebSocketBackend.connectand makeHttp4sWSConnection.closeInternalperform an actual teardown. - Add
Http4sWebSocketBackendSuiteto verify release/onClose are triggered exactly once across repeatedclose()calls. - Update JSON serialization to use Circe
noSpacesand expand CI/automation config for Temurin JDK 25.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scalajs/src/main/scala/clue/js/WebSocketJsBackend.scala | Use asJson.noSpaces for outbound WS JSON payloads. |
| scalajs/src/main/scala/clue/js/FetchJsBackend.scala | Use asJson.noSpaces for GraphQL POST bodies. |
| http4s/src/test/scala/clue/http4s/Http4sWebSocketBackendSuite.scala | Add regression coverage for deterministic close/release behavior. |
| http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala | Implement guarded lifecycle management and non-no-op close semantics. |
| build.sbt | Bump base version, add JDK 25 to workflow matrix, and add MUnit deps to http4s module. |
| .scalafix.conf | Configure OrganizeImports Scala 3 dialect. |
| .mergify.yml | Update required CI status checks to include Temurin 25 jobs. |
| .github/workflows/ci.yml | Run CI matrix on Temurin 25 + 17 (with rootJS only on 25) and shift formatting/lint/mima/doc gates to 25. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5b74e10 to
c552a77
Compare
hugo-vrijswijk
approved these changes
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ApolloClient.disconnect()did not actually close http4s/JDK WebSocket connections becauseHttp4sWSConnection.closeInternalwas a no-op. That left receive fibers/resources alive, causing teardown hangs and unstable close behavior on newer JDK HttpClient WS implementations (notably JDK 21/25).Connection lifecycle / teardown in
Http4sWebSocketBackendconnectto make teardown deterministic and idempotent:releaseguardhandler.onCloseguardSucceeded/Errored/Canceledexits.Real close behavior in
Http4sWSConnectionHttp4sWSConnectionnow owns:closeInternalnow actively tears down the connection by canceling the listener and executing guarded close/release actions instead of returningunit.Focused regression coverage
Http4sWebSocketBackendSuiteto assert explicit disconnect semantics:close()calls release the underlying high-level WS resource exactly onceonClosecallback is emitted exactly onceOriginal prompt
Create a draft pull request implementing proper WebSocket close semantics in repo
gemini-hlsw/clue.Context
http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala, classHttp4sWSConnection.closeInternalis currently a no-op (Concurrent[F].unit // TODO).Http4sWebSocketBackend.connectusesconnectHighLevel(...).allocatedand starts a fiber to drainconnection.receiveStream, and only callsreleasein the stream finalizer.IllegalArgumentException: statusCodeon close.User request
fix-ws-close.Goals
ApolloClient.disconnect()actually closes the socket when usingHttp4sWebSocketBackend.Http4sWebSocketBackend.connectare cleaned up deterministically on disconnect.releasefrom.allocatedis always invoked.handler.onClosecallbacks, but ensure they are triggered appropriately during explicit client disconnect.Suggested implementation direction (agent may adjust as needed after exploring repo/docs)
Http4sWebSocketBackend.connect, manage lifecycle withResource/allocatedCaseor store:releaseactionHttp4sWSConnection.closeInternalto:releaseonFinalizeCaseruns; but also guard against duplicateonClosecalls.Acceptance criteria
References
http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scalacore/src/main/scala/clue/websocket/ApolloClient.scala(disconnect callsconnection.closeInternal)Branch
fix-ws-close.The following is the prior conversation context from the user's chat exploration (may be truncated):
User: ```
20:22:00.632 [INFO ] org.http4s.ember.server.EmberServerBuilderCompanionPlatform - Ember-Server service bound to address: 127.0.0.1:8080
20:22:00.764 [DEBUG] lucuma-odb-test - GET web socket: Request(method=GET, uri=/ws, httpVersion=HTTP/1.1, headers=Headers(Connection: Upgrade, Host: 127.0.0.1:8080, Upgrade: websocket, User-Agent: Java-http-client/25.0.2, Sec-WebSocket-Key: QgjjxBT0IVwNGjHyw7+EIw==, Sec-WebSocket-Protocol: graphql-transport-ws, Sec-WebSocket-Version: 13))
20:22:00.792 [DEBUG] lucuma-odb-test - Received Text frame (last=true) from client: {
"type" : "connection_init",
"payload" : {
"Authorization" : "Bearer bob"
}
}
20:22:00.808 [DEBUG] lucuma-odb-test - received ConnectionInit(Some(object[Authorization -> "Bearer bob"]))
20:22:00.902 [DEBUG] lucuma-odb-test - Subscriptions send Some(Right(ConnectionAck(None))) enqueued
20:22:00.902 [DEBUG] lucuma-odb-test - Subscriptions send Some(Right(Ping(None))) enqueued
20:22:00.906 [DEBUG] lucuma-odb-test - Sending to client: {
"type" : "connection_ack"
}
20:22:00.908 [DEBUG] lucuma-odb-test - Sending Ping
20:22:00.923 [DEBUG] lucuma-odb-test - Received Text frame (last=true) from client: {
"type" : "subscribe",
"id" : "query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624",
"payload" : {
"query" : "query { foo }"
}
}
20:22:00.924 [DEBUG] lucuma-odb-test - received Subscribe(query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624,GraphQLRequest(query { foo },None,None,None))
20:22:00.968 [DEBUG] lucuma-odb-test - Subscriptions send Some(Right(Next(query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624,GraphQLResponse(Right({
"foo" : 42
}),None)))) enqueued
20:22:00.968 [DEBUG] lucuma-odb-test - Subscriptions send Some(Right(Complete(query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624))) enqueued
20:22:00.970 [DEBUG] lucuma-odb-test - Sending to client: {
"type" : "next",
"id" : "query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624",
"payload" : {
"data" : {
"foo" : 42
}
}
}
20:22:00.971 [DEBUG] lucuma-odb-test - Sending to client: {
"type" : "complete",
"id" : "query-foo-6cc1dc8d-f2ba-4224-a287-39f373098624"
}
20:22:05.795 [DEBUG] lucuma-odb-test - Sending Ping
20:22:10.792 [DEBUG] lucuma-odb-test - Sending Ping
Test run lucuma.graphql.routes.AuthSuite s...
This pull request was created from Copilot chat.