Fix flaky Netty request-timeout metrics tests#5381
Merged
Conversation
The "properly update metrics when a request times out" tests asserted gauge values after a fixed Thread.sleep, racing the asynchronous metrics update which only happens when the endpoint's (non-interruptible) logic completes - ~1s after the 503 timeout response for the Future server. The ~100ms margin was not enough on loaded CI machines. Replace the fixed sleeps with `eventually` and a widened patience config (15s / 150ms), the same idiom used in ServerMetricsTest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…imeout-test-flakiness
adamw
added a commit
that referenced
this pull request
Jul 7, 2026
## Problem The "Send and receive SSE" test in `AkkaHttpServerTest` flaked on CI (seen on PR #5381's CI run, 2026-07-07, ci 2.13 / JVM 11) with only: ``` org.scalatest.exceptions.TestFailedException was thrown. (AkkaHttpServerTest.scala:82) ``` No message, no cause. An identical sibling test exists in `PekkoHttpServerTest`. Two things went wrong: 1. **Swallowed exception**: the test ended with `.transform(sse => sse shouldBe ..., ex => fail(ex))`. `fail(ex)` wraps the real exception as the *cause* of a message-less `TestFailedException`, and the test reporter prints neither — hence the bare failure line. 2. **The actual flake**: the sttp client backend was created with default `ConnectionPoolSettings`, whose `response-entity-subscription-timeout` is **1s**. The test consumes the response via `asStreamUnsafe` and only materializes the SSE-parsing `runFold` in a later future callback. On a loaded CI runner that gap can exceed 1s, at which point the akka/pekko-http client pool fails the entity stream ("Response entity was not subscribed after 1 second..."). ## Fix - Build the per-test client backend with `ConnectionPoolSettings(actorSystem).withResponseEntitySubscriptionTimeout(1.minute)` so a slow runner can't hit the subscription timeout. - Replace the `.transform(..., ex => fail(ex))` with a plain `.map(...)` so if the test ever fails again, the real exception (message + type) propagates to the report instead of a bare `TestFailedException`. - Close the per-test client backend after the request completes (it was previously leaked). Same changes applied to both the akka and pekko variants. ## Force-repro proof - A `-Dakka.http.host-connection-pool.response-entity-subscription-timeout=1ms` override propagates into the (non-forked) test JVM but does *not* reproduce locally: on an idle machine the stream is subscribed before the scheduler-tick-granularity timeout task can fire. - Simulating CI load instead — inserting `Thread.sleep(1500)` before the `runFold` materialization on unmodified master (default 1s timeout) — reproduces the **exact** CI symptom: `TestFailedException was thrown. (AkkaHttpServerTest.scala:82)`. - The same 1500ms delay with this fix applied: test **passes**. ## Verification - Both SSE tests (akka + pekko) run 3x each: all green. - `akkaHttpServer2_12/Test/compile`, `pekkoHttpServer2_12/Test/compile`, `pekkoHttpServer3/Test/compile`: all pass. - Full `AkkaHttpServerTest` suite: 301 tests, 0 failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Problem
The test
properly update metrics when a request times out(inNettyFutureRequestTimeoutTests, and its sync counterpart inNettySyncRequestTimeoutTests) is flaky on CI, failing intermittently with1 was not equal to 0at theactiveRequests.get() shouldBe 0assertion. It failed on three dependency-update PRs in the week of 2026-07-06 alone (runs for #5379, #5376 and #5373).The root cause is a race in the test, not in production code: the metrics are only decremented when the endpoint's logic completes, which for the Future-based server happens ~1 second after the 503 timeout response is received (the
Futurerunning the logic is not interruptible). The test used a fixedThread.sleep(1100), leaving only a ~100 ms margin before asserting — not enough on a loaded CI machine. The sync test had an even tighter fixedThread.sleep(100).Fix
Replace the fixed sleeps with
eventually { ... }and a widened patience config (15 s timeout / 150 ms interval) — the same idiom already used for the same reason inServerMetricsTest. This asserts eventual consistency instead of racing the asynchronous metrics update. No production code is changed.Verification
Ran locally:
sbt "nettyServer/testOnly sttp.tapir.server.netty.NettyFutureServerTest -- -z \"properly update metrics\""— passedsbt "nettyServerSync3/testOnly sttp.tapir.server.netty.sync.NettySyncServerTest -- -z \"properly update metrics\""— passed🤖 Generated with Claude Code