diff --git a/docs/decisions.md b/docs/decisions.md index cbfce90..b39885f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -761,16 +761,12 @@ rule and an index of the titles below. round trip before the bind is answered, so gating on `closed` let a send arriving in that window go out unbound and come back `ESME_RINVBNDSTS`. `LinkGate` owns the answer instead — `shut(returning)` on every teardown, `open()` only once `comeBackUp()` has a bound link — and - `OutgoingRequests.canCarry()` reads it rather than `closed`. The bind itself cannot wait for what it - creates, so `pastDrain()` lets the three bind commands past the gate and the window, the same door - `unbind()` takes through `now()`. The gate is told what happened and never reads back into the - session: a collaborator that has to ask does not own its decision, which is how the first cut ended - up answering the same question two different ways at admit and at release. For the same reason the - retry in `pastDrain()` asks `gate.isUp()` rather than `canCarry()`, which also reads the socket — a - condition that loops on something the gate does not gate on spins against a gate that admits it - straight back. `LinkGate.returning` is a copy of `retrying()` taken at teardown, and stays true - only because nothing stops the reconnect loop without `emitClose()` following it: `drain()` and - `end()` are the only callers of `stop()`. A third caller has to shut the gate itself. + `OutgoingRequests.canCarry()` reads it rather than `closed`. The gate is told what happened and + never reads back into the session: a collaborator that has to ask does not own its decision, which + is how the first cut ended up answering the same question two different ways at admit and at + release. The retry in `carry()` asks `gate.awaitsNextLink()` rather than `canCarry()`, which also + reads the socket: a loop condition the gate does not gate on spins against a gate that admits it + straight back. ## Internals and tests diff --git a/src/link-gate.ts b/src/link-gate.ts index 6711790..e3fe884 100644 --- a/src/link-gate.ts +++ b/src/link-gate.ts @@ -42,6 +42,11 @@ export class LinkGate { return this.up; } + /** Shut, with another link on its way to reopen it. */ + awaitsNextLink(): boolean { + return !this.up && this.returning; + } + /** Why the gate will never admit a request, or undefined while one may still get through. */ refusal(): Error | undefined { return this.up || this.returning ? undefined : over(); diff --git a/src/outgoing-requests.ts b/src/outgoing-requests.ts index 80939d5..2193f4a 100644 --- a/src/outgoing-requests.ts +++ b/src/outgoing-requests.ts @@ -93,11 +93,11 @@ export class OutgoingRequests { return Promise.resolve({ err: new Error('Session is shutting down') }); } - return this.pastDrain(input, options); + return this.carry(input, options); } - /** The same path without that refusal, which a receipt for a held message has to take. */ - async pastDrain( + /** The same path without the drain's refusal, which a receipt for a held message has to take. */ + async carry( input: PduObjectInput, options: SendOptions, ): Promise> { @@ -125,8 +125,7 @@ export class OutgoingRequests { const attempt = await this.attempt(input, options).finally(() => { this.window.release(); }); - // Nothing reached the socket, so the next link carries it instead of the caller resending. - if (!attempt.retryOnNextLink || this.gate.isUp() || this.gate.refusal()) return attempt.result; + if (!this.retriesOnNextLink(attempt)) return attempt.result; } } @@ -151,6 +150,12 @@ export class OutgoingRequests { return { err: new Error(`Shut down with ${String(unfinished)} request(s) unfinished`) }; } + /** Nothing reached the socket, so the next link carries it instead of the caller resending. */ + private retriesOnNextLink(attempt: Attempt): boolean { + // Until the gate is shut it admits the retry straight back onto the dead socket, and the loop spins. + return attempt.retryOnNextLink && this.gate.awaitsNextLink(); + } + /** Why a request cannot go out at all, as opposed to not yet. */ private refuse(input: PduObjectInput, options: SendOptions): Error | undefined { // Before the gate and the window, or an aborted call waits for what it will never use. diff --git a/src/session.ts b/src/session.ts index 3a3b011..4b337f7 100644 --- a/src/session.ts +++ b/src/session.ts @@ -261,7 +261,7 @@ export class Session extends EventEmitter { reportError: err => { this.emit('sessionError', err); }, reportMessageDlr: merged => { this.emit('messageDlr', merged); }, send: input => this.send(input), - sendPastDrain: input => this.outgoing.pastDrain(input, {}), + sendPastDrain: input => this.outgoing.carry(input, {}), smsListeners: () => this.listenerCount('sms'), }; } @@ -395,6 +395,7 @@ export class Session extends EventEmitter { else this.emitClose(); } + // Copied into the gate at teardown, so stopping the loop anywhere but drain() and end() has to shut the gate too. private retrying(): boolean { return this.reconnectLoop !== undefined && !this.reconnectLoop.isStopped(); } diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index fb11cad..09c315c 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1448,6 +1448,18 @@ describe('LinkGate', () => { assert.match(held.err?.message ?? '', /Aborted while waiting for a link/); }); + + test('awaits the next link only while shut with one on its way', () => { + const gate = new LinkGate({ log: silentLog, timeout: 100 }); + + assert.equal(gate.awaitsNextLink(), false, 'up'); + gate.shut(true); + assert.equal(gate.awaitsNextLink(), true, 'shut, returning'); + gate.open(); + assert.equal(gate.awaitsNextLink(), false, 'reopened'); + gate.shut(false); + assert.equal(gate.awaitsNextLink(), false, 'shut for good'); + }); }); describe('SendWindow', () => { diff --git a/todo.md b/todo.md index 94da552..be7f61f 100644 --- a/todo.md +++ b/todo.md @@ -199,11 +199,6 @@ next work ([decision](docs/decisions.md#internals-and-tests)). ### Locality — next, ahead of everything below; 5–6 today, and the gate is 7 -- [ ] **Name `pastDrain()`'s retry condition and what makes the loop end.** The exit is a - three-term disjunction over two collaborators, whose comment covers the first term only, and - the method is named for what it bypasses. Do not change what it asks: `gate.isUp()` rather than - `canCarry()` is deliberate and recorded. - - [ ] **Replace `resolveBody`'s `settles` boolean with the decision it stands for.** One boolean chooses both whether to overwrite `data_coding` and which params to read it from, across four helpers all named some abstraction of "body". Return a named source — `'short_message' | @@ -326,7 +321,8 @@ next work ([decision](docs/decisions.md#internals-and-tests)). - [ ] **Make `LinkGate.isUp()`'s doc true or its state match it.** It says a link attached but not yet bound cannot carry a request, while `up` starts `true`, so the first link and a server - session are up before any bind. From the comprehension panel of #25. + session are up before any bind. The gate decision in `docs/decisions.md` makes the same claim + in its title, and carries the same fix. From the comprehension panel of #25. - [ ] **Move `checkSessionOptions()`'s doc comment to what it describes.** It explains why a count below 1 is refused, which is `checkLimits`' job, and says nothing of the function it heads.