Skip to content

test(qwp): tolerate early terminal error ACK in regression test - #111

Merged
jerrinot merged 1 commit into
mainfrom
fix/server-error-ack-terminal-race
Oct 15, 2026
Merged

jerrinot merged 1 commit into
mainfrom
fix/server-error-ack-terminal-race

Conversation

@jerrinot

@jerrinot jerrinot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

The server's terminal error ACK can arrive before the first flush() returns, making ServerErrorAckTerminalTest fail on valid sender behavior. Tolerate LineSenderException from that initial flush while retaining the assertions that the error stays terminal and no reconnect attempts occur.

Fixes #110.

@jerrinot

Copy link
Copy Markdown
Contributor Author

Review of PR #111 at level 1: approve. It changes only the test, and the fix is correct.

The PR wraps the first flush() in try/catch (LineSenderException). That is safe because the latched error keeps firing:

  • CursorWebSocketSendLoop.checkError() rethrows the same latched exception every time it is called.
  • table() reaches that check through checkNotClosed, so the next call still throws the same exception.
  • The assertions that matter are unchanged and still catch the regression this test exists for: zero reconnect attempts, the next call throws, and the message mentions the rejection.

The changed test class passed 5 out of 5 runs on the PR head (dfc52d81) locally on JDK 25.

Critical

None.

Moderate

None.

Minor

  • Problem: Fixes #110. sits at the bottom of the PR description instead of the top, which is where the convention puts it.
  • Net impact: None for users; it's a PR description convention only.
  • Evidence: the PR body.

Coverage map

No production code changed, so every row is exempt. The test gate passes with no coverage gaps. No tandem PR is needed: it's a unit-test-only change, and searching questdb and questdb-enterprise for the branch found nothing.

Summary

  • Verdict: approve.
  • Submodules: no submodule pointers changed.
  • Findings: 0 Critical, 0 Moderate, 1 Minor, all inside the diff.
  • Scope: at level 1 I checked how callers depend on the latched error (flush → table → close), not every usage of the sender.

@jerrinot jerrinot added the READY label Oct 15, 2026
@jerrinot
jerrinot merged commit ac4a5d5 into main Oct 15, 2026
19 checks passed
@jerrinot
jerrinot deleted the fix/server-error-ack-terminal-race branch October 15, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: ServerErrorAckTerminalTest.testServerErrorAckIsTerminalAndDoesNotBurnReconnectBudget

2 participants