Skip to content

test(fpc): gate WebSocket EAGAIN regression - #564

Merged
regyssilveira merged 2 commits into
HashLoad:masterfrom
freitasjca:test/fpc-regression-suites
Sep 10, 2026
Merged

test(fpc): gate WebSocket EAGAIN regression#564
regyssilveira merged 2 commits into
HashLoad:masterfrom
freitasjca:test/fpc-regression-suites

Conversation

@freitasjca

@freitasjca freitasjca commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an FPC + epoll WebSocket regression test for the EAGAIN handling fixed in #549 and runs it in GitHub Actions with FPC 3.2.2.

The test opens a real WebSocket connection over the epoll provider and verifies that:

  1. the first EAGAIN does not resume the HTTP pipeline or emit a stray response;
  2. an idle upgraded connection remains silent and open across multiple read ticks;
  3. a frame sent after the idle period reaches OnMessage and is echoed;
  4. an abrupt peer close releases the read loop and the server continues serving HTTP.

Review adjustments

The maintainer follow-up commit makes the original contribution CI-ready:

  • wires the regression runner into .github/workflows/tests.yml;
  • binds the server to 127.0.0.1;
  • handles partial socket writes;
  • accumulates complete HTTP headers and bodies across multiple recv calls;
  • captures errno once before classifying EAGAIN/EWOULDBLOCK;
  • enables fail-fast shell behavior;
  • removes the duplicate Pascal keep-alive test, retaining the existing FPCHttpKeepAliveServer.dpr + fpc_keepalive_regression.py pair as the authoritative keep-alive regression gate.

Validation

  • updated test against current code: 5 passed, 0 failed;
  • control run against commit 37d699f (before the WebSocket fix): 2 passed, 3 failed, detecting the stray HTTP response, idle-connection violation, and missing echo;
  • workflow validated with actionlint.

Two standalone FPC programs covering transports the DUnitX suite cannot reach,
since it is Delphi-only. Neither depends on a test framework, and each exits
non-zero on failure.

FPCHttpKeepaliveTest.dpr drives 30 sequential requests over one reused
connection and asserts each completes within 35 ms. KeepConnection := True is
required: TFPHTTPClient defaults it to False, and the stall only appears on a
reused connection because Linux quickack makes the first ACK on every socket
immediate. It refuses to build below FPC 3.3.1, where the provider's keep-alive
and TCP_NODELAY code is not compiled at all. Verified against 3.3.4 on trunk:
max 1 ms.

tests/fpc-websocket/ covers the EAGAIN path on the epoll transport. It asserts
SILENCE after the first EAGAIN rather than liveness, which is the distinction
that catches the defect. 5/5 on FPC 3.2.2 against the reworked implementation
in 3.3.4.

Additive only: no existing file is modified except .gitignore, which gains two
entries for FPC test binaries whose Linux builds have no extension.
@regyssilveira

Copy link
Copy Markdown
Contributor

Thank you for adding coverage for these FPC-specific paths. The WebSocket regression test was compiled and run with FPC 3.2.2 in Docker and all 5 checks passed.

Before merging, could you please address the following points?

  1. Wire the new tests into GitHub Actions. As submitted, neither program runs automatically, so they do not yet act as regression gates.
  2. Please revisit the keep-alive timing assertion. Requiring every one of 30 requests to complete within 35 ms is vulnerable to unrelated runner scheduling spikes. The existing Python regression test ignores connection setup and evaluates steady-state median latency; the Pascal test should use a similarly robust criterion, or replace/consolidate the existing test rather than duplicate it.
  3. In the WebSocket test, read the HTTP response until the complete headers and declared body have arrived. A single recv is allowed to return a partial response and can make the final /ping assertion flaky.
  4. Bind the test server explicitly to 127.0.0.1 instead of all interfaces.
  5. Please clarify the intended role of FPCHttpKeepaliveTest.dpr relative to the existing FPCHttpKeepAliveServer.dpr + fpc_keepalive_regression.py pair. Keeping one authoritative CI path will reduce maintenance and contradictory thresholds.

Issue #565 identifies a separate Windows/FPC gap: keep-alive is enabled there, while the TCP_NODELAY socket handler is currently Unix-only. We will handle that implementation separately, but Windows CI coverage for the resulting behavior would be welcome either here or in a follow-up.

@regyssilveira regyssilveira changed the title test(fpc): keep-alive and WebSocket EAGAIN regression tests test(fpc): gate WebSocket EAGAIN regression Sep 10, 2026

@regyssilveira regyssilveira left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Validated after the maintainer follow-up: 5/5 checks pass on FPC 3.2.2 in Docker, the pre-fix control fails the three expected assertions, and the workflow passes actionlint. The duplicate keep-alive test was removed in favor of the existing authoritative CI regression.

@regyssilveira
regyssilveira merged commit a267de4 into HashLoad:master Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants