stdb: hold frames issued before the socket opens #113

Merged

yuris merged 5 commits from fix/hold-pre-open-frames into main 2026-08-22 11:38:30 +00:00

Conversation 0 Commits 5 Files changed 2 +231 -13

yuris commented 2026-08-22 11:05:54 +00:00

Owner

Copy link

Calling the SDK on the line after connect crashed with an error message from a
native module the caller did not write:

ERROR:SCRIPT: stdb/init.lua:464: Connection isn't connected

connect returns before the websocket handshake completes, but the send closure
was installed immediately after websocket.connect returned, so a frame issued
in that window reached extension-websocket's own LuaSend guard
(websocket.cpp:562), which raises for any state that is not STATE_CONNECTED.

This was the only mistake in this SDK that did not explain itself. Forgetting
the websocket dependency, leaving off ws://, and using a stale generated
module all produce messages naming the actual problem; this one named neither
the SDK nor readiness. init.lua's READINESS note documented the constraint,
but documentation only helps someone who read it before writing the line, and
this is the line people write on instinct.

What changed

Frames issued before EVENT_CONNECTED are held and flushed, in order, when it
arrives.

Nothing competes for that window:

• settle_pending re-sends nothing. ANSWERED_BY_REPLAY means stay quiet,
  something else will re-issue this — not re-issue it.
• resubscribe_all runs off the InitialConnection frame, which by definition
  arrives on an open socket.

The caller is tracked either way: connection.lua:747 parks every request in
pending before calling send, so a socket that dies without ever opening
still settles each held call through the same "connection lost" sweep as any
other drop. The invariant at connection.lua:876 continues to hold.

The flush is guarded because HTML5 delivers EVENT_CONNECTED twice for one
socket — the emscripten onopen handler fires it directly
(emscripten_callbacks.cpp:11) and the shared update loop fires it again off
the same state transition (websocket.cpp:871). Unguarded, the second flush
would re-send what the first one sent.

ready/outbox are per-socket locals inside open(), so frames held for a
socket that never opened cannot leak onto its replacement.

Verification

End to end, real engine, live 2.x server. A throwaway edit to
dev/example-chat moved a subscribe_query to the line after connect, then
reverted.

Before:

DEBUG:SCRIPT: EXAMPLE: starting
ERROR:SCRIPT: stdb/init.lua:464: Connection isn't connected
DEBUG:SCRIPT: EXAMPLE: disconnected: 0
DEBUG:SCRIPT: EXAMPLE: FAIL timeout

The raise also killed the socket, so the run reconnected and then timed out.

After:

DEBUG:SCRIPT: EXAMPLE: starting
DEBUG:SCRIPT: EXAMPLE: connected
DEBUG:SCRIPT: EXAMPLE: EARLY-OK: a pre-open subscribe applied
DEBUG:SCRIPT: EXAMPLE: subscribed; 1 message(s) cached
DEBUG:SCRIPT: EXAMPLE: PASS

This is what settles the one thing static reading could not: EVENT_CONNECTED
does arrive in-engine, and the held frame flushes and gets answered.

Unit tests. Four new assertions in test_init.lua pin the hold, the flush,
the order, and the double-EVENT_CONNECTED guard. Confirmed to fail without the
fix:

FAIL dev/test/test_init.lua: FAIL nothing reaches the socket before it opens: expected 0, got 2
12 passed, 1 failed

With the fix: 13 passed, 0 failed. stylua --check clean.

The existing connect sends BINARY frames test now opens the socket before
asserting, since the frame no longer reaches the binding synchronously.

Behaviour change worth noting

A too-early call's error moves from an immediate raise at the callsite to the
handle or callback it already returns. The outcome still arrives — that is the
connection.lua:876 invariant — but the stack trace at the offending line is
gone. For the beginner this fix targets that is a trade rather than a
regression, and it is the reason READINESS in init.lua was rewritten rather
than deleted: "connected" is still where anything depending on identity
belongs.

Calling the SDK on the line after `connect` crashed with an error message from a native module the caller did not write: ``` ERROR:SCRIPT: stdb/init.lua:464: Connection isn't connected ``` `connect` returns before the websocket handshake completes, but the send closure was installed immediately after `websocket.connect` returned, so a frame issued in that window reached extension-websocket's own `LuaSend` guard (`websocket.cpp:562`), which raises for any state that is not `STATE_CONNECTED`. This was the only mistake in this SDK that did not explain itself. Forgetting the websocket dependency, leaving off `ws://`, and using a stale generated module all produce messages naming the actual problem; this one named neither the SDK nor readiness. `init.lua`'s `READINESS` note documented the constraint, but documentation only helps someone who read it before writing the line, and this is the line people write on instinct. ## What changed Frames issued before `EVENT_CONNECTED` are held and flushed, in order, when it arrives. Nothing competes for that window: - `settle_pending` re-sends nothing. `ANSWERED_BY_REPLAY` means *stay quiet, something else will re-issue this* — not *re-issue it*. - `resubscribe_all` runs off the `InitialConnection` frame, which by definition arrives on an open socket. The caller is tracked either way: `connection.lua:747` parks every request in `pending` **before** calling `send`, so a socket that dies without ever opening still settles each held call through the same `"connection lost"` sweep as any other drop. The invariant at `connection.lua:876` continues to hold. The flush is guarded because HTML5 delivers `EVENT_CONNECTED` **twice** for one socket — the emscripten `onopen` handler fires it directly (`emscripten_callbacks.cpp:11`) and the shared update loop fires it again off the same state transition (`websocket.cpp:871`). Unguarded, the second flush would re-send what the first one sent. `ready`/`outbox` are per-socket locals inside `open()`, so frames held for a socket that never opened cannot leak onto its replacement. ## Verification **End to end, real engine, live 2.x server.** A throwaway edit to `dev/example-chat` moved a `subscribe_query` to the line after `connect`, then reverted. Before: ``` DEBUG:SCRIPT: EXAMPLE: starting ERROR:SCRIPT: stdb/init.lua:464: Connection isn't connected DEBUG:SCRIPT: EXAMPLE: disconnected: 0 DEBUG:SCRIPT: EXAMPLE: FAIL timeout ``` The raise also killed the socket, so the run reconnected and then timed out. After: ``` DEBUG:SCRIPT: EXAMPLE: starting DEBUG:SCRIPT: EXAMPLE: connected DEBUG:SCRIPT: EXAMPLE: EARLY-OK: a pre-open subscribe applied DEBUG:SCRIPT: EXAMPLE: subscribed; 1 message(s) cached DEBUG:SCRIPT: EXAMPLE: PASS ``` This is what settles the one thing static reading could not: `EVENT_CONNECTED` does arrive in-engine, and the held frame flushes and gets answered. **Unit tests.** Four new assertions in `test_init.lua` pin the hold, the flush, the order, and the double-`EVENT_CONNECTED` guard. Confirmed to fail without the fix: ``` FAIL dev/test/test_init.lua: FAIL nothing reaches the socket before it opens: expected 0, got 2 12 passed, 1 failed ``` With the fix: `13 passed, 0 failed`. `stylua --check` clean. The existing `connect sends BINARY frames` test now opens the socket before asserting, since the frame no longer reaches the binding synchronously. ## Behaviour change worth noting A too-early call's error moves from an immediate raise at the callsite to the handle or callback it already returns. The outcome still arrives — that is the `connection.lua:876` invariant — but the stack trace at the offending line is gone. For the beginner this fix targets that is a trade rather than a regression, and it is the reason `READINESS` in `init.lua` was rewritten rather than deleted: `"connected"` is still where anything depending on identity belongs.

yuris added 1 commit 2026-08-22 11:05:54 +00:00

stdb: hold frames issued before the socket opens ... 06e7964579

`connect` returns before the websocket handshake completes, but the send
closure was installed immediately after `websocket.connect` returned. A
call issued on the next line therefore reached extension-websocket's
LuaSend guard, which raises for any state that is not STATE_CONNECTED:

  ERROR:SCRIPT: stdb/init.lua:464: Connection isn't connected

That message comes from a native module the caller did not write and
names neither this SDK nor readiness — the one class of mistake here
that did not explain itself. Reproduced end to end against a live 2.x
server: the raise also killed the socket, so the run then reconnected
and timed out.

Frames issued before EVENT_CONNECTED are now held and flushed, in
order, when it arrives. Nothing competes for that window:
`settle_pending` re-sends nothing, and `resubscribe_all` runs off the
InitialConnection frame, which by definition arrives on an open socket.
The caller is tracked either way, because connection.lua parks every
request in `pending` before calling `send`, so a socket that dies
without ever opening still settles each held call through the same
"connection lost" sweep as any other drop.

The flush is guarded because HTML5 delivers EVENT_CONNECTED twice for
one socket: the emscripten onopen handler fires it directly and the
shared update loop fires it again off the same state transition.

`ready`/`outbox` are per-socket locals inside `open()`, so frames held
for a socket that never opened cannot leak onto its replacement.

yuris referenced this pull request from a commit 2026-08-22 11:21:14 +00:00

stdb: do not flush a held outbox after close, and narrow the promise

yuris added 1 commit 2026-08-22 11:21:15 +00:00

stdb: do not flush a held outbox after close, and narrow the promise ... 1425e24051

Both from review of #113.

FLUSH AFTER CLOSE. On HTML5, `db:close()` during the handshake left the
outbox flushable. `CloseConnection` gates its entire body on
STATE_CONNECTED and the unconditional transition to STATE_DISCONNECTED
is compiled only under HAVE_WSLAY (websocket.cpp:165-186), so closing
mid-handshake is a no-op there; the browser still fires onopen,
EVENT_CONNECTED arrives, and the new branch flushed. `close` has
already told every held caller "connection lost" via settle_pending, so
those frames reached the wire after the SDK reported them lost — a
reducer executing server-side after `close()` returned. Native was
unaffected: CloseConnection transitions there, so EVENT_CONNECTED never
arrives.

`closing` now also stops the send path from HOLDING. After close
nothing will flush an outbox again, so a held frame would strand its
caller in `pending` with no outcome on any surface — #41's failure mode,
and strictly worse than the raise a dead socket already gives.

THE PROMISE WAS TOO WIDE. The rewritten READINESS docstring turned a
prohibition into an unqualified guarantee, which invites a consumer to
drop their own readiness gate. Only the pre-open window is held. After a
drop, the gap until the replacement socket exists is not: `set_send`
installs a new holder only when `open()` runs, i.e. after the reconnect
backoff, so a send during the backoff reaches the dead socket's closure
and raises "Invalid connection". Reconnect is on by default, so that is
the common path. Holding it instead would strand the caller whenever no
socket ever replaces this one, so the docstring states the boundary
rather than moving it.

Both new assertions confirmed to fail without their guard.

yuris referenced this pull request from a commit 2026-08-22 11:27:58 +00:00

stdb: key the hold on _state, not closing, and fix the gating advice

yuris added 1 commit 2026-08-22 11:27:58 +00:00

stdb: key the hold on _state, not closing, and fix the gating advice ... fbcad0d7bb

From the second review pass of #113.

`not closing` covered only one of the two terminal states. `closing` is
set by `close` alone, while a drop with `reconnect = false` is equally
terminal — `schedule_reconnect` takes its terminal branch, sweeps
`pending`, and leaves `closing` false. A request issued after that was
still HELD, in an outbox nothing would ever flush, parked in `pending`
after the final sweep: no frame, no callback, no error, `is_pending()`
true forever. #41 verbatim, and worse than the raise it replaced.

`_state == "closed"` is exactly "no socket will ever open again" — it is
written only by `schedule_reconnect`'s terminal branch and by `close` —
so both the flush guard and the send closure now use it. Strict superset
of the old predicate, so nothing it covered is lost.

Also from the review: the narrowed READINESS docstring told consumers to
gate on `db:state()`, which fails in the one place they would most
likely gate. `"disconnected"` and `"socket_error"` are both emitted
BEFORE `schedule_reconnect` moves the state, so `state()` still reads
"connected" inside the handler being told about the drop. The docstring
now names the events as the boundary and quotes both raise messages the
extension can produce rather than only one.

The first attempt at the regression test fired EVENT_CONNECTED before
the drop, which set `ready` and made the send take the direct path for
the wrong reason — it passed against the unfixed guard. It now never
opens the socket, and fails with `not closing` restored.

yuris referenced this pull request from a commit 2026-08-22 11:33:37 +00:00

stdb: take both terminal tests, since neither implies the other

yuris added 1 commit 2026-08-22 11:33:37 +00:00

stdb: take both terminal tests, since neither implies the other ... a758c4837f

From the third review pass of #113, which caught a regression the second
pass's own recommendation introduced.

`_state` is written "closed" only by the two terminal places, but it is
NOT STICKY there: `credentials` writes `_state = "connected"` on any
InitialConnection (connection.lua:972), and nothing stops a closed
connection decoding frames — `dispatch` and `handle_frame` have no
closed guard, and init.lua feeds EVENT_MESSAGE straight in.

That is reachable on the HTML5 zombie path this PR already documents:
`close()` mid-handshake does not close the socket, so onopen fires, the
flush guard correctly suppresses the flush, and then the server's first
frame resurrects `_state` to "connected" while the connection is closed.
`ready` is still false, so every later send was HELD again — #41
verbatim, on the very path fbcad0d thought it had covered.

Neither predicate subsumes the other. `closing` is write-once-true but
set only by `close`, so it misses a drop with `reconnect = false`.
`_state` covers that drop but can be resurrected. Both guards now take
both tests.

Four mutations, four distinct failures, one per guard:

  drop `not closing`   -> the resurrected-_state assertion
  drop `_state`        -> the terminal-drop assertion
  drop both            -> the send-after-close assertion
  delete flush guard   -> the close-before-open assertion

Also from the review: `state()` inside a drop handler reads whatever it
read BEFORE the drop, which is "connecting" for a socket that never came
up, not always "connected"; and the claim that `state()` is accurate
everywhere else was false in exactly the zombie case above, so it is
gone rather than qualified.

The underlying defect — a closed connection still decoding frames, so it
re-emits "connected", re-persists credentials and refills the cache —
is broader than this PR and wants its own issue. Not folded in here.

yuris added 1 commit 2026-08-22 11:38:24 +00:00

test: use the method form for dispatch, and name the third drop-handler state ... 7390574926

Both cosmetic, from the fourth review pass. `zombie:dispatch(...)` matches
the file's four pre-existing sites; the READINESS parenthetical enumerated
two of the three values `state()` can hold inside a drop handler, and
"reconnecting" is the third, on a replacement socket that failed its own
handshake.

yuris merged commit f08c078318 into main 2026-08-22 11:38:30 +00:00

yuris referenced this pull request from a commit 2026-08-22 11:38:31 +00:00

stdb: hold frames issued before the socket opens (#113)

yuris referenced this pull request from a commit 2026-08-22 11:40:17 +00:00

release: v4.0.1

yuris referenced this pull request 2026-08-22 11:40:27 +00:00

release: v4.0.1 #114

yuris referenced this pull request 2026-08-22 11:41:23 +00:00

A closed connection still decodes frames, resurrecting _state #115

yuris referenced this pull request 2026-08-22 13:36:00 +00:00

fix(stdb): drop frames on a closed connection #116

yuris referenced this pull request from a commit 2026-08-22 13:56:33 +00:00

fix(stdb): drop frames on a closed connection (#116)

yuris referenced this pull request from a commit 2026-08-22 13:57:06 +00:00

fix(stdb): drop frames on a closed connection (#116)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:defold

Defold engine integration

  

area:docs

Documentation and examples

  

area:infra

Flake, CI, secretspec

  

area:protocol

Wire protocol conformance

  

area:testing

Test harness and fixtures

  

kind:bug

Something broken

  

kind:chore

Maintenance

  

phase:breadth

UnsubscribeMulti, procedures, BSATN

  

phase:core

Core SDK layers (protocol/cache/connection)

  

phase:hardening

Reconnect, token persistence, errors

  

phase:slice

Vertical slice: first playable end-to-end

No labels

area:defold

area:docs

area:infra

area:protocol

area:testing

kind:bug

kind:chore

phase:breadth

phase:core

phase:hardening

phase:slice

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

yuris/spacetimedb-defold!113

No description provided.