mirror of
https://github.com/clusterio/clusterio.git
synced 2026-11-14 07:24:52 +09:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
5e0ab3423f
commit
3512042cf3
2 files changed
+54
-2
No files matched your search
@@ -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<T>(event: Event<T>, dst: libData.Address) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user