Skip to content

Implement deterministic WebSocket close semantics in Http4s client backend#772

Merged
hugo-vrijswijk merged 6 commits into
mainfrom
copilot/fix-ws-close
May 8, 2026
Merged

Implement deterministic WebSocket close semantics in Http4s client backend#772
hugo-vrijswijk merged 6 commits into
mainfrom
copilot/fix-ws-close

Conversation

Copilot AI commented May 7, 2026

Copy link
Copy Markdown
Contributor

ApolloClient.disconnect() did not actually close http4s/JDK WebSocket connections because Http4sWSConnection.closeInternal was 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 Http4sWebSocketBackend

    • Added explicit lifecycle state in connect to make teardown deterministic and idempotent:
      • one-time resource release guard
      • one-time handler.onClose guard
    • Listener finalization now always routes through guarded close+release behavior across Succeeded / Errored / Canceled exits.
  • Real close behavior in Http4sWSConnection

    • Http4sWSConnection now owns:
      • receive listener fiber
      • cancel-close callback action
      • release action
    • closeInternal now actively tears down the connection by canceling the listener and executing guarded close/release actions instead of returning unit.
  • Focused regression coverage

    • Added Http4sWebSocketBackendSuite to assert explicit disconnect semantics:
      • repeated close() calls release the underlying high-level WS resource exactly once
      • onClose callback is emitted exactly once
override def closeInternal(closeParameters: Option[CloseParams]): F[Unit] =
  listener.cancel >> onCanceled >> releaseNow
Original prompt

Create a draft pull request implementing proper WebSocket close semantics in repo gemini-hlsw/clue.

Context

  • Current issue: WebSocket client disconnect does not actually close the underlying connection.
  • In http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala, class Http4sWSConnection.closeInternal is currently a no-op (Concurrent[F].unit // TODO).
  • Http4sWebSocketBackend.connect uses connectHighLevel(...).allocated and starts a fiber to drain connection.receiveStream, and only calls release in the stream finalizer.
  • This leads to leaked connections/fibers, test timeouts, and in newer JDK HttpClient WebSocket implementations (JDK 21/25) errors like IllegalArgumentException: statusCode on close.

User request

  • Open a PR on a new branch fix-ws-close.
  • Mention that it was requested by @hugo-vrijswijk.
  • Keep PR as draft.

Goals

  1. Implement real close behavior so ApolloClient.disconnect() actually closes the socket when using Http4sWebSocketBackend.
  2. Ensure resources/fibers started in Http4sWebSocketBackend.connect are cleaned up deterministically on disconnect.
  3. Avoid hanging receive fibers and ensure release from .allocated is always invoked.
  4. Preserve existing behavior for message handling and handler.onClose callbacks, but ensure they are triggered appropriately during explicit client disconnect.

Suggested implementation direction (agent may adjust as needed after exploring repo/docs)

  • In Http4sWebSocketBackend.connect, manage lifecycle with Resource/allocatedCase or store:
    • the listener fiber
    • the release action
    • a "closed" signal to prevent double-close
  • Implement Http4sWSConnection.closeInternal to:
    • attempt to send a close frame if possible (use available http4s WSConnectionHighLevel APIs)
    • otherwise at least cancel the listener fiber and invoke release
    • be idempotent
  • Ensure that closing the connection causes the receive stream to complete, so onFinalizeCase runs; but also guard against duplicate onClose calls.

Acceptance criteria

  • Unit/integration tests should no longer hang due to leaked websocket connections.
  • Disconnect triggers closure promptly.
  • PR includes clear explanation in description, including mention of JDK 21/25 stricter close handling and that this PR addresses the root cause (no-op close).

References

  • File: http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala
  • File: core/src/main/scala/clue/websocket/ApolloClient.scala (disconnect calls connection.closeInternal)

Branch

  • Create and use 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.

@hugo-vrijswijk

Copy link
Copy Markdown
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>
@mergify mergify Bot added the http4s label May 7, 2026
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
Copilot AI requested a review from hugo-vrijswijk May 7, 2026 19:31
@hugo-vrijswijk hugo-vrijswijk marked this pull request as ready for review May 8, 2026 08:17
@mergify mergify Bot added the scalajs label May 8, 2026
@hugo-vrijswijk hugo-vrijswijk requested a review from Copilot May 8, 2026 08:51
@hugo-vrijswijk hugo-vrijswijk changed the title [WIP] Implement deterministic WebSocket close semantics in Http4s backend (requested by @hugo-vrijswijk) Implement deterministic WebSocket close semantics in Http4s backend (requested by @hugo-vrijswijk) May 8, 2026
@hugo-vrijswijk hugo-vrijswijk changed the title Implement deterministic WebSocket close semantics in Http4s backend (requested by @hugo-vrijswijk) Implement deterministic WebSocket close semantics in Http4s backend May 8, 2026
@hugo-vrijswijk hugo-vrijswijk changed the title Implement deterministic WebSocket close semantics in Http4s backend Implement deterministic WebSocket close semantics in Http4s client backend May 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.connect and make Http4sWSConnection.closeInternal perform an actual teardown.
  • Add Http4sWebSocketBackendSuite to verify release/onClose are triggered exactly once across repeated close() calls.
  • Update JSON serialization to use Circe noSpaces and 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.

Comment thread http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala
Comment thread http4s/src/main/scala/clue/http4s/Http4sWebSocketBackend.scala Outdated
Comment thread http4s/src/test/scala/clue/http4s/Http4sWebSocketBackendSuite.scala
@hugo-vrijswijk hugo-vrijswijk force-pushed the copilot/fix-ws-close branch from 5b74e10 to c552a77 Compare May 8, 2026 09:27
@hugo-vrijswijk hugo-vrijswijk merged commit 60435df into main May 8, 2026
14 checks passed
@hugo-vrijswijk hugo-vrijswijk deleted the copilot/fix-ws-close branch May 8, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants