Name carry()'s retry condition and what ends its loop #39
+6
-10
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<Result<{ pduObj: PduObject }>> {
|
||||
@@ -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.
|
||||
|
||||
+2
-1
@@ -261,7 +261,7 @@ export class Session extends EventEmitter<SessionEvents> {
|
||||
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<SessionEvents> {
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user