From 3512042cf3e21cd07b1cf05671f2981459e0db8b Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 23 Jul 2026 22:58:37 +0000 Subject: [PATCH 1/2] Forget a forwarded request that could not be sent forwardRequest records a request so that its response can be sent back the way it came, but recorded it before handing the message to the connector. When the connector threw, because the session to the next hop was gone, the record stayed behind for a request that had never left. Routers answer the origin themselves when forwarding throws, so the request was already accounted for. The leftover record then had it answered a second time when the link cleared its pending requests on disconnect, producing a duplicate response and "Received response N without a pending request" at the far end. Remove the record when the forward throws. Doing it here rather than in the routers covers both the host, which catches and responds, and the controller router, which does not. Fixes #650 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/lib/src/link/link.ts | 14 ++++++++++-- test/lib/link/link.js | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/lib/src/link/link.ts b/packages/lib/src/link/link.ts index b7bc8c02..c44798e7 100644 --- a/packages/lib/src/link/link.ts +++ b/packages/lib/src/link/link.ts @@ -566,8 +566,18 @@ export class Link { src: message.src, dst: message.dst, }; - this._forwardedRequests.set(message.src.requestIndex(), pending); - this.connector.forward(message); + const requestIndex = message.src.requestIndex(); + this._forwardedRequests.set(requestIndex, pending); + try { + this.connector.forward(message); + } catch (err) { + // The request never left, so nothing will answer it. Callers + // respond with the error themselves, and leaving it recorded here + // would have it answered a second time when pending requests are + // cleared. + this._forwardedRequests.delete(requestIndex); + throw err; + } } sendEvent(event: Event, dst: libData.Address) { diff --git a/test/lib/link/link.js b/test/lib/link/link.js index c8bd06c4..dd380b59 100644 --- a/test/lib/link/link.js +++ b/test/lib/link/link.js @@ -504,6 +504,48 @@ describe("lib/link/link", function() { }); }); + describe(".forwardRequest()", function() { + // A request is recorded so that its response can be sent back the + // way it came. If it never left there is nothing to send back, and + // leaving it recorded has it answered twice, see #650. + function forwardingSetup() { + const originConnector = new mock.MockConnector(dst, src); + const origin = new lib.Link(originConnector); + const nextHopConnector = new mock.MockConnector(src, dst); + const nextHop = new lib.Link(nextHopConnector); + const message = new lib.MessageRequest(1, src, dst, "SimpleRequest"); + return { origin, originConnector, nextHop, nextHopConnector, message }; + } + + it("should record a request it forwarded", function() { + const { origin, nextHop, nextHopConnector, message } = forwardingSetup(); + nextHop.forwardRequest(message, origin); + assert.equal(nextHop.pendingRequestCount, 1); + assert.equal(nextHopConnector.sentMessages.length, 1); + }); + + it("should not record a request it failed to forward", function() { + const { origin, nextHop, nextHopConnector, message } = forwardingSetup(); + nextHopConnector.send = () => throwSimple("Session Closed"); + + assert.throws(() => nextHop.forwardRequest(message, origin), { message: "Session Closed" }); + assert.equal(nextHop.pendingRequestCount, 0, "failed forward was left pending"); + }); + + it("should not answer a request it failed to forward a second time", function() { + const { origin, originConnector, nextHop, nextHopConnector, message } = forwardingSetup(); + nextHopConnector.send = () => throwSimple("Session Closed"); + + // Routers answer the origin themselves when forwarding throws. + assert.throws(() => nextHop.forwardRequest(message, origin), { message: "Session Closed" }); + originConnector.sendResponseError(new lib.ResponseError("Session Closed"), message.src); + const answers = originConnector.sentMessages.length; + + nextHop._clearPendingRequests(new lib.SessionLost("Session Lost")); + assert.equal(originConnector.sentMessages.length, answers, "request was answered twice"); + }); + }); + describe(".snoopEvent()", function() { it("should snoop an event", function() { let handled = false; From 48c07f9ab3f936ba006fe5221b81510e436b887d Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:26:40 +0100 Subject: [PATCH 2/2] Make explanation comment less verbose --- packages/lib/src/link/link.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/lib/src/link/link.ts b/packages/lib/src/link/link.ts index c44798e7..d399024d 100644 --- a/packages/lib/src/link/link.ts +++ b/packages/lib/src/link/link.ts @@ -571,10 +571,7 @@ export class Link { try { this.connector.forward(message); } catch (err) { - // The request never left, so nothing will answer it. Callers - // respond with the error themselves, and leaving it recorded here - // would have it answered a second time when pending requests are - // cleared. + // Clean up pending request as there will be no response this._forwardedRequests.delete(requestIndex); throw err; }