chore(typescript)!: release 0.4.0 and pin the same-key correlation residual

CHANGELOG.md

@@ -2,12 +2,19 @@
 
 ## [Unreleased]
 
-### TypeScript SDK — Breaking (npm package `@freenetorg/freenet-stdlib`; next release must be 0.4.0, not a patch)
+### TypeScript SDK 0.4.0 — Breaking (npm package `@freenetorg/freenet-stdlib`)
 
 The npm package is versioned separately from the Rust crate. This release
 changes runtime behavior for existing callers, so on a 0.x line it takes a
 minor bump: **0.3.0 -> 0.4.0**, not 0.3.1.
 
+Upgrading from npm: 0.3.0 and earlier matched responses to requests by *arrival
+order alone*, with no correlation of any kind, so concurrent requests for
+different contracts could resolve into each other's promises. That is what this
+release fixes. Applications that serialised their requests to work around it can
+stop doing so **for requests on different contracts**; see "Known limitation"
+below before relaxing it for concurrent requests on the *same* contract.
+
 - **Host responses are now correlated to requests by contract key.**
   `FreenetWsApi` previously resolved the *oldest* pending request of a type
   with whatever response arrived, with no correlation at all. Two concurrent
@@ -46,6 +53,44 @@ minor bump: **0.3.0 -> 0.4.0**, not 0.3.1.
   names. An error naming no pending contract still fails everything, since it
   may be connection-wide and must not leave callers waiting out the timeout.
 
+#### Known limitation: two requests for the SAME contract key
+
+Correlation is by contract key, because the key is the only identifying field a
+`ContractResponse` carries. Two requests for the *same* key are therefore
+indistinguishable, and one case remains open, reported as
+[#96](https://github.com/freenet/freenet-stdlib/issues/96):
+
+Giving up on a request locally does not stop the node working on it — the SDK
+sends nothing to cancel the operation — so a request that hit
+`REQUEST_TIMEOUT_MS` can still be answered afterwards. If the caller has retried
+the same contract by then, that late answer matches the retry exactly and
+settles it. The retry resolves with a result fetched for a request its caller
+already abandoned, and its own answer is later dropped against an empty queue.
+Mostly this means staler state for the right contract, but not always: a retry
+issued with `fetchContract: true` can be settled by an earlier response that
+carries no contract.
+
+**This release does not fix that, deliberately.** A client-side fence was
+attempted and withdrawn: with no request id on the wire, the SDK cannot tell a
+late answer from the retry's own answer, so any rule that drops "the next
+response for this key" is as likely to drop the retry's answer as the ghost's.
+Doing so hangs that retry until its own timeout, which mints another
+indistinguishable case — a self-sustaining chain of spurious timeouts against a
+contract the node is serving correctly. That is a worse failure than the
+mis-delivery it was meant to prevent. The evidence is on
+[#105](https://github.com/freenet/freenet-stdlib/pull/105).
+
+The real fix is a client-generated request id echoed in every terminal response,
+which makes correlation exact and this whole class of problem unreachable. That
+is a wire-protocol change spanning freenet-core, tracked in
+[#106](https://github.com/freenet/freenet-stdlib/issues/106).
+
+**Until then**, an application that issues concurrent requests for the same
+contract key, or retries one after a timeout, should treat a response as
+"an answer for this contract" rather than "the answer to this call": re-check
+whatever the result is used for, and prefer idempotent retries. Requests for
+*different* contracts are correlated correctly and need no such care.
+
 ### Breaking (next release must be 0.9.0, not a patch)
 - **`DelegateRequest::RegisterDelegateWithPredecessors`** removed (added in
   0.8.4). freenet-core's node-side handler for this request was disabled in

typescript/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@freenetorg/freenet-stdlib",
-  "version": "0.3.0",
+  "version": "0.4.0",
   "description": "Freenet standard library and utils",
   "main": "dist/src/index.js",
   "types": "dist/src/index.d.ts",

typescript/tests/correlation.test.ts

@@ -673,3 +673,96 @@ describe("request timeout", () => {
     expect((await putB).key.encode()).toEqual(ENCODED_B);
   });
 });
+
+/**
+ * The residual that key-based correlation cannot close, pinned as a KNOWN
+ * LIMITATION rather than as desired behaviour.
+ *
+ * Two requests for the same contract key are identical on the wire — the key is
+ * the only identifying field a `ContractResponse` carries — so a late answer to
+ * a request the caller has already abandoned matches a retry for that key
+ * exactly. Reported as issue 96.
+ *
+ * A client-side fence for this was attempted and withdrawn (PR 105). With no
+ * request id, "drop the next response for this key" cannot tell the ghost's
+ * answer from the retry's own, and dropping the retry's answer hangs it until
+ * its own timeout — which mints another indistinguishable case, chaining
+ * spurious timeouts against a contract the node is serving correctly. That is
+ * worse than the mis-delivery it removes.
+ *
+ * These tests therefore assert what the SDK ACTUALLY DOES today. They are
+ * expected to be inverted, not deleted, by the wire-level request id that fixes
+ * this properly (issue 106).
+ */
+describe("known limitation: same-key requests are indistinguishable", () => {
+  const RESIDUAL_WS_URL = "ws://localhost:1243/contract/command/";
+  let server: Server;
+
+  beforeEach(() => {
+    server = new Server(RESIDUAL_WS_URL);
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+    server.clients().forEach((c) => c.close());
+    server.close();
+  });
+
+  async function connectResidual(): Promise<FreenetWsApi> {
+    const api = new FreenetWsApi(new URL(RESIDUAL_WS_URL), makeHandler());
+    await settle();
+    return api;
+  }
+
+  test("a late answer to a timed-out get settles a retry for the same key", async () => {
+    const api = await connectResidual();
+
+    jest.useFakeTimers();
+    const abandoned = api
+      .get(new GetRequest(new ContractKey(KEY_A), false))
+      .catch((e) => e);
+    jest.advanceTimersByTime(31_000);
+    expect(((await abandoned) as Error).message).toEqual("Request timeout");
+
+    // The node is still working on the abandoned request; nothing cancelled it.
+    const retry = api.get(new GetRequest(new ContractKey(KEY_A), false));
+    jest.useRealTimers();
+
+    // The abandoned request's answer arrives. It matches the retry's key
+    // exactly, and nothing on the wire distinguishes the two requests.
+    server
+      .clients()
+      .forEach((c) => c.send(getResponseFor(KEY_A, [0x01])));
+
+    // KNOWN LIMITATION: the retry settles with the abandoned request's answer.
+    // Invert this assertion once responses carry a request id.
+    expect((await retry).state).toEqual([0x01]);
+  });
+
+  test("a response for a DIFFERENT key never settles the wrong request", async () => {
+    const api = await connectResidual();
+
+    jest.useFakeTimers();
+    const abandoned = api
+      .get(new GetRequest(new ContractKey(KEY_A), false))
+      .catch((e) => e);
+    jest.advanceTimersByTime(31_000);
+    await abandoned;
+
+    const other = api.get(new GetRequest(new ContractKey(KEY_B), false));
+    jest.useRealTimers();
+
+    // The bound that DOES hold: the abandoned request's answer cannot reach a
+    // request for another contract. This is the half #94 fixed and the half
+    // that carried the real user-visible harm.
+    server
+      .clients()
+      .forEach((c) => c.send(getResponseFor(KEY_A, [0x01])));
+    expect(await statusOf(other)).toEqual("pending");
+
+    server
+      .clients()
+      .forEach((c) => c.send(getResponseFor(KEY_B, [0x02])));
+    expect((await other).state).toEqual([0x02]);
+  });
+});