Procedure and one-off-query callbacks are silently lost when the socket dies #41

New issue

Closed

opened 2026-08-19 17:27:25 +00:00 by yuris · 0 comments

yuris commented

2026-08-19 17:27:25 +00:00

Owner

Found while actioning the review on #29 (PR #38). Out of scope there, and left deliberately.

The problem

PR #38 fixed this for reducer calls: when the socket dies, set_send walks the in-flight Call handles and settles each with error = "connection lost", so a caller learns their request will never complete.

Procedures and one-off queries got no such treatment, because they have no handle to settle. Their c.pending entries are dropped when the socket is replaced, and the callback is never invoked. A consumer that wrote:

db:call_procedure("echo", { "hi" }, function(err, value)
  -- never called if the socket dies mid-flight
end)

waits forever for a callback that cannot arrive. That is exactly the shape of the bug #38 closed for call_reducer, still open on two other paths.

Why it matters more than a stuck table entry

The leaked c.pending entry is minor — it is one table per lost request, bounded by how many requests were in flight. The real cost is the silent non-completion: a game that disables a button until a procedure returns leaves it disabled forever, and nothing anywhere reports why. This SDK's whole documented posture is that failures should be loud or absent.

Two ways to fix it

Handles for procedures and one-off queries, matching #29's design — call_procedure returns something with on_ok / on_error, and set_send settles it like a Call. Consistent, and it makes the whole request surface uniformly observable. Larger, and it adds public API that #24 freezes.
Settle the plain callbacks directly in set_send: walk every remaining c.pending entry regardless of kind and invoke its callback with an error. Much smaller, no new API, and it fits the existing (err, ...) contract. The catch is that call_reducer's legacy callback deliberately does not fire on a dead socket (PR #38 asserts that, because it never used to and the shape is frozen), so doing it for procedures would make the two paths inconsistent in the opposite direction.

Option 2 is probably right for 1.0 — smaller, no frozen surface added — but the inconsistency with call_reducer's legacy callback has to be a deliberate, documented choice rather than an accident. Whichever is chosen, say why the other was not.

M.subscribe (the legacy whole-set form) parks pending entries in c.subscriptions and resubscribe_all re-issues them with a throwaway function() end callback. So the legacy subscribe form has no handle and no way to observe a rejection — strictly less observable than subscribe_query. That is deliberate, not a bug, but it belongs in the same conversation if anyone proposes handles for M.subscribe.

Acceptance

A test proving a call_procedure and a one_off_query in flight when the socket dies both reach their callback with an error, and that c.pending is empty afterwards. Mutation-check it: removing the settle must make the test hang or fail, not pass quietly.

Found while actioning the review on #29 (PR #38). Out of scope there, and left deliberately. ## The problem PR #38 fixed this for reducer calls: when the socket dies, `set_send` walks the in-flight `Call` handles and settles each with `error = "connection lost"`, so a caller learns their request will never complete. **Procedures and one-off queries got no such treatment**, because they have no handle to settle. Their `c.pending` entries are dropped when the socket is replaced, and the callback is never invoked. A consumer that wrote: ```lua db:call_procedure("echo", { "hi" }, function(err, value) -- never called if the socket dies mid-flight end) ``` waits forever for a callback that cannot arrive. That is exactly the shape of the bug #38 closed for `call_reducer`, still open on two other paths. ## Why it matters more than a stuck table entry The leaked `c.pending` entry is minor — it is one table per lost request, bounded by how many requests were in flight. The real cost is the silent non-completion: a game that disables a button until a procedure returns leaves it disabled forever, and nothing anywhere reports why. This SDK's whole documented posture is that failures should be loud or absent. ## Two ways to fix it 1. **Handles for procedures and one-off queries**, matching #29's design — `call_procedure` returns something with `on_ok` / `on_error`, and `set_send` settles it like a `Call`. Consistent, and it makes the whole request surface uniformly observable. Larger, and it adds public API that #24 freezes. 2. **Settle the plain callbacks directly** in `set_send`: walk every remaining `c.pending` entry regardless of `kind` and invoke its callback with an error. Much smaller, no new API, and it fits the existing `(err, ...)` contract. The catch is that `call_reducer`'s legacy callback deliberately does **not** fire on a dead socket (PR #38 asserts that, because it never used to and the shape is frozen), so doing it for procedures would make the two paths inconsistent in the opposite direction. Option 2 is probably right for 1.0 — smaller, no frozen surface added — but the inconsistency with `call_reducer`'s legacy callback has to be a deliberate, documented choice rather than an accident. Whichever is chosen, say why the other was not. ## Related asymmetry, same area `M.subscribe` (the legacy whole-set form) parks pending entries in `c.subscriptions` and `resubscribe_all` re-issues them with a throwaway `function() end` callback. So the legacy subscribe form has no handle and no way to observe a rejection — strictly less observable than `subscribe_query`. That is deliberate, not a bug, but it belongs in the same conversation if anyone proposes handles for `M.subscribe`. ## Acceptance A test proving a `call_procedure` and a `one_off_query` in flight when the socket dies both reach their callback with an error, and that `c.pending` is empty afterwards. Mutation-check it: removing the settle must make the test hang or fail, not pass quietly.

yuris referenced this issue

2026-08-19 18:26:34 +00:00

v1.0.0 release checklist #24

yuris referenced this issue

2026-08-20 01:45:51 +00:00

v1.0.0 release checklist #24

yuris referenced this issue from a commit

2026-08-20 01:59:05 +00:00

connection: settle handle-free requests when the socket dies (#41)

yuris referenced this issue from a pull request that will close it,

2026-08-20 02:00:31 +00:00

connection: settle handle-free requests when the socket dies #47