Keep an ended link ended, and pin the drop a listener closes over
Mirror / push (push) Successful in 6s
Test / lint (pull_request) Successful in 23s
Test / test (18) (pull_request) Successful in 32s
Test / test (20) (pull_request) Successful in 31s
Test / test (22) (pull_request) Successful in 32s
Test / test (24) (pull_request) Successful in 32s
Test / test (26) (pull_request) Successful in 32s

This commit is contained in:
2026-09-28 21:36:31 +02:00
parent fe320d23a8
commit 242eb30ab3
4 changed files with 49 additions and 4 deletions
+3 -3
View File
@@ -676,7 +676,7 @@ rule and an index of the titles below.
the peer, whose every request is bounded by `responseTimeout` unless the caller set that to 0 as the peer, whose every request is bounded by `responseTimeout` unless the caller set that to 0 as
well, and unsafe for the application, which nothing bounds — `close()` is what you reach for when well, and unsafe for the application, which nothing bounds — `close()` is what you reach for when
the application is stuck, so it may not block on the application coming unstuck. That half falls the application is stuck, so it may not block on the application coming unstuck. That half falls
back to `responseTimeout`, the same answer the link gate's hold already takes — and to that back to `responseTimeout`, the same answer `LinkLife`'s hold already takes — and to that
option's default where it is 0 as well, since neither option is an answer about the application. option's default where it is 0 as well, since neither option is an answer about the application.
- **What the application holds unanswered is capped on constants, and a message past the cap is - **What the application holds unanswered is capped on constants, and a message past the cap is
@@ -738,7 +738,7 @@ rule and an index of the titles below.
optional so every construction site answers. `UnansweredError` stays unexported: `unanswered` is optional so every construction site answers. `UnansweredError` stays unexported: `unanswered` is
the one spelling on the public surface. The hold is bounded by `responseTimeout` rather than an the one spelling on the public surface. The hold is bounded by `responseTimeout` rather than an
option of its own — that is already the answer to how long one request may wait — and its clock option of its own — that is already the answer to how long one request may wait — and its clock
starts when the send is issued rather than when it first finds the gate shut, so one budget covers starts when the send is issued rather than when it first finds the link down, so one budget covers
every hold a single call makes. That timer is the one here that is not `unref()`'d: a held request every hold a single call makes. That timer is the one here that is not `unref()`'d: a held request
is awaited with the socket already destroyed, so an unref'd one lets a process whose only remaining is awaited with the socket already destroyed, so an unref'd one lets a process whose only remaining
work is that send exit without settling it. work is that send exit without settling it.
@@ -757,7 +757,7 @@ rule and an index of the titles below.
window is this end's own concurrency draining as the peer answers rather than a link going nowhere, window is this end's own concurrency draining as the peer answers rather than a link going nowhere,
and that bound would fail a message with more segments than `maxOutstanding` partway through and that bound would fail a message with more segments than `maxOutstanding` partway through
against a slow peer. The failure is a plain `Error` rather than `UnansweredError`, the same answer against a slow peer. The failure is a plain `Error` rather than `UnansweredError`, the same answer
an abort at the gate already gives. The drain half needs nothing: `close({ signal })` already hands the signal to an abort while held for a link already gives. The drain half needs nothing: `close({ signal })` already hands the signal to
`window.idle()`, and `unbind()` taking none is the shape README states. `window.idle()`, and `unbind()` taking none is the shape README states.
- **`LinkLife` decides whether a link can carry a request, and a bind is what makes it one.** - **`LinkLife` decides whether a link can carry a request, and a bind is what makes it one.**
+3 -1
View File
@@ -91,8 +91,10 @@ export class LinkLife {
return () => this.wait(deadline, signal); return () => this.wait(deadline, signal);
} }
/** A socket from the reconnect loop, not yet bound. */ /** A socket from the reconnect loop, not yet bound. An ended session stays ended. */
attach(): void { attach(): void {
if (this.isOver()) return;
this.phase = 'binding'; this.phase = 'binding';
} }
+36
View File
@@ -1486,6 +1486,9 @@ describe('LinkLife', () => {
link.end(); link.end();
assert.match((await held).err?.message ?? '', /Session is closed/); assert.match((await held).err?.message ?? '', /Session is closed/);
link.attach();
assert.equal(link.isAttached(), false, 'ended is final');
assert.equal(link.end(), false);
}); });
}); });
@@ -2458,6 +2461,39 @@ describe('a peer that sends the next segment only once the last one is answered'
assert.deepEqual(await peerOf(smpp).close(), {}, 'a group nothing completed is not held'); assert.deepEqual(await peerOf(smpp).close(), {}, 'a group nothing completed is not held');
}); });
test('reports a drop once as disconnected when a listener closes the session over the segments it lost', async t => {
const smpp = await startServer(t);
const { session } = await connect(t, smpp, { reconnect: { maxDelay: 100, minDelay: 20 } });
assert.ok(session);
const events: string[] = [];
const closed = once<true>(resolve => { session.on('close', () => { resolve(true); }); });
session.on('close', () => { events.push('close'); });
session.on('disconnected', () => { events.push('disconnected'); });
session.on('sessionError', () => { void session.close(); });
const [first] = segmentsOf(0x2D);
assert.ok(first);
const delivered = await peerOf(smpp).send({
cmdName: 'deliver_sm',
params: submitSmParams(
{ from: '46701113311', message: text, to: '46709771337' },
first,
{ encoding: 'ASCII', multipart: true },
),
});
assert.equal(delivered.err, undefined);
await peerOf(smpp).close();
await closed;
assert.deepEqual(events, ['disconnected', 'close']);
});
test('close() still waits for a concatenated message the application has not answered', async t => { test('close() still waits for a concatenated message the application has not answered', async t => {
const smpp = await startServer(t, { shutdownTimeout: 50 }); const smpp = await startServer(t, { shutdownTimeout: 50 });
const incoming = once<Sms>(resolve => { const incoming = once<Sms>(resolve => {
+7
View File
@@ -211,6 +211,13 @@ hardest, and the held-message flow across `incoming-requests.ts`, `held-messages
### Correctness ### Correctness
- [ ] **Refuse to open a link that dropped while its rebind was answered.** A peer sending
`bind_resp` and FIN together can tear the link down before `comeBackUp()` resumes; it then
calls `link.open()` on a `down` link, `resetTimers()` skips, and `attempt()` reports success,
so the session is `up` on a destroyed socket with nothing to reconnect it until `close()`.
`open()` accepting only `binding`, and `comeBackUp()` returning an err when the link is no
longer attached, lets the loop retry. Unreproduced; from the stability review of #48.
- [ ] **Register a multipart send's receipt merge before its segments go out.** `Session.sendSms()` - [ ] **Register a multipart send's receipt merge before its segments go out.** `Session.sendSms()`
calls `dlrMerger.expect()` only once `submitSms()` resolves, after the last segment's response, calls `dlrMerger.expect()` only once `submitSms()` resolves, after the last segment's response,
so a receipt for an early segment that arrives first is logged at `debug` as naming no merge, so a receipt for an early segment that arrives first is logged at `debug` as naming no merge,