Skip to content

prove: #1052 race cases fail without the early-message buffer - #1077

Closed
chappie-daemon wants to merge 7 commits into
mainfrom
prove/1052-early-buffer-removed
Closed

chappie-daemon wants to merge 7 commits into
mainfrom
prove/1052-early-buffer-removed

Conversation

@chappie-daemon

Copy link
Copy Markdown
Collaborator

Temporary. Reverts the #1045 buffering (early listener + pause) under the new socket-race features, to show they fail for the reason they name. Closed and deleted once CI reports.

tirthraj01 and others added 4 commits November 7, 2026 09:34
The cases all send at the open tick, but beforeEach stubs the admission check
to resolve immediately, so the adapter installs its normal listener before any
frame can arrive and the buffering path is never exercised. holdAdmission()
leaves the check pending until the frames are in the server's queue, then
admits; every case now goes through the replay path.

Shape 1 also asserted only that each socket saw some REQ. It now requires
exactly one frame per socket, the full command shape, and the multiset of
subscription ids received to equal the ids sent.
Issue #1052 asks for the reply a client is entitled to, not survival. The
adapter spec stops at frame delivery, so it cannot see a dropped REQ or a
subscription left live by a racing CLOSE. These six features drive a real app
over a real socket and assert EOSE, OK and NOTICE:

- 10 sockets subscribing at the open tick, 20 rounds, exactly one EOSE each
- two REQs in one turn answered in order
- REQ and CLOSE in one turn: no event on the closed id, live subscription
  still receives, socket still answers
- EVENT at the open tick: OK with ok=true
- fragmented and binary frames honoured, next command still answered
- invalid JSON refused with a NOTICE, socket usable afterwards
onClientMessage does not make a later message wait for an earlier one, and
each subscribe handler sends EOSE when its own query finishes, so two replies
can arrive in either order. Both must arrive; their order is not the relay's
to promise. after-frames stays last in the framing case because it is sent
only after the other two have answered.
@changeset-bot

changeset-bot Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cc60d98

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
nostream Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@coveralls

coveralls commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 42425271318

Coverage decreased (-2.4%) to 75.221%

Details

  • Coverage decreased (-2.4%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 331 coverage regressions across 38 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

331 previously-covered lines in 38 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
src/utils/relay-probe/ws-protocol-probe.ts 101 11.15%
src/scripts/export-events.ts 30 67.53%
src/controllers/admission/get-admission-check-controller.ts 20 9.62%
src/import-events.ts 16 64.82%
src/adapters/web-socket-server-adapter.ts 14 72.84%
src/utils/admin-health.ts 14 50.91%
src/factories/web-app-factory.ts 11 57.69%
src/controllers/invoices/get-invoice-status-controller.ts 10 12.0%
src/handlers/request-handlers/rate-limiter-middleware.ts 9 17.5%
src/repositories/event-repository.ts 8 94.1%

Coverage Stats

Coverage Status
Relevant Lines: 13150
Covered Lines: 10342
Line Coverage: 78.65%
Relevant Branches: 6907
Covered Branches: 4745
Branch Coverage: 68.7%
Branches in Coverage %: Yes
Coverage Strength: 688.72 hits per line

💛 - Coveralls

Without a delay the admission check answers before a frame written at the
handshake can reach the server, so the cases passed even with the early-message
buffer removed — the race they name never happened. Admission is a Redis round
trip in production; hold it for 50 ms for these scenarios only, which is what
the #1045 unit case does with its own unresolved promise.
@chappie-daemon

Copy link
Copy Markdown
Collaborator Author

Proof complete: 135 scenarios, 6 failed (the six @socket-race cases, each timing out on the reply it names), 129 passed. That is the sensitivity evidence the issue asks for. Closing and deleting the branch.

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.

3 participants