Cap a read deadline at setTimeout's ceiling instead of firing at once

[BeeHiveTeam]

Closes #100

Both read deadlines clamped timeoutMs the same way — src/wallet.mjs:79 for NFT reads and :292 for history:

const deadline = Number.isFinite(requested) && requested > 0 ? requested : NFT_TIMEOUT_MS;

A finite number above 2^31 - 1 passes that check and setTimeout does not accept it: Node warns Timeout duration was set to 1 and fires after a millisecond. The meaning inverts — a caller asking for effectively no limit gets the request cancelled at once, which is what the clamp's own comment says it exists to prevent. A sub-millisecond value does the same from the other end: 0.5 passes > 0 and rounds up to 1 ms.

The failure is not deterministic, and saying so matters more than the numbers. A one-millisecond timer races the loopback response, so the same call rejects or succeeds run to run. Fifty consecutive calls with timeoutMs = 2147483648 against a fixture answering immediately, on 0d2c880:

path	rejected	answered in time
getNfts	36 of 50	14
getHistory	19 of 50	31

Those ratios are one machine's, and the gap between the two paths is structural rather than noise: getHistory only rejects when both requests lose the race (if (!fulfilled.length)), while getNfts has a single request to lose. What does not vary at all is the cause: TimeoutOverflowWarning … Timeout duration was set to 1 on every call, whichever side wins.

What changed

The rule moves into resolveDeadline, exported and pure, and both reads call it. It has three outcomes rather than two: a usable duration is floored and used, an oversized finite one is capped at MAX_TIMEOUT_MS, and anything that names no usable duration takes the default. Both defaults are untouched.

Infinity belongs in the third group, not with the capped values, and that distinction is the point of the guard rather than a detail of it. Capping it would hand back a 24.9-day wait — the unbounded read #92 and #93 exist to remove, wearing a deadline's clothes. A finite number is a duration the caller named, however unreasonable; Infinity is the absence of one.

Both fetch wrappers resolve their own argument now. Before this, fetchReservoir clamped inside itself while getHistory clamped at the caller and handed fetchExplorerItems a number that was already resolved. Two shapes around one rule is exactly how the next copy picks the wrong one — the mechanism this change is about — so leaving it inside the change would have been odd. Three lines, no behaviour change: Infinity → 10000, 2147483648 → 2147483647, 1.9 → 1, 0.5 → 10000, NaN → 10000, 250 → 250, before and after. The timeout message quotes the resolved deadline either way, which the existing stall tests pin.

The floor is there because the deadline is quoted back in the timeout message. 1.9 used to pass through and be reported as timed out after 1.9ms, while setTimeout truncates it: over 60 runs each, 1.9 fires when 1 does — median 1.05 ms against 1.05 ms — while 2 lands at 2.05 ms. Absolute numbers drift with the scheduler; the step is what holds. That is the 2^31 problem in miniature: a number told to the operator that no timer honoured. Flooring makes the number reported the number used.

Exported rather than kept private because the rule cannot be asserted honestly through a request. With the bug the timer races the response, as the table above shows. With the fix the deadline is twenty-four days, so a test that waits for it to fire never finishes. Neither is a test; the arithmetic is.

An intermediate version of this change dropped the Number.isFinite check, on the grounds that the cap handled Infinity too and mutation testing showed the check unobservable. Both halves were wrong: the cap did handle Infinity, but handling it that way was the bug above, and the check was unobservable only because the table asserting Infinity said CEILING. Restoring the check, the mutation is caught. Kept as a lesson about what "no test observes this" can mean — sometimes the tests are right and the behaviour is wrong; sometimes the assertion is simply wrong too.

Tests

405, up from 386. 10 consecutive runs, no failure.

• resolveDeadline as a table: the ceiling itself, one past it, far past it, Number.MAX_SAFE_INTEGER, Infinity, exactly 1 ms, a fraction above one, a larger fraction, a fractional string, below 1 ms, zero, negative, NaN, a word, null, undefined, a numeric string;
• the sub-millisecond values sit with the fallbacks rather than with the cap, which is where they belong: setTimeout rounds them up, so taken literally they cancel everything like NaN does, not wait a shorter time;
• the existing stall tests on both paths already pin that the message quotes the deadline actually used, so the wiring from the clamp to the operator's error needed nothing new;
• the per-path tables now say only what they can observe, down to the wording. A fixture that answers at once cannot tell ten seconds from twenty-four days, so their message changed from must fall back to the default to must not cancel the request — which is what those rows actually prove: that the path routes through the helper rather than handing setTimeout the raw value. Which outcome each value produces is pinned in the resolveDeadline table instead.

Mutation-checked, each guard reverted on its own with a no-op edit as a control:

reverted	result
resolveDeadline returns the raw request	17 red
Number.isFinite removed, so Infinity is capped	1 red
the floor removed	3 red
the cap removed	3 red
>= 1 back to > 0	4 red
ceiling raised by one	3 red
NFT wrapper bypasses the helper	1 red
explorer wrapper bypasses the helper	1 red
control: no-op edit	405 pass

One thing I corrected in review rather than shipping: the new constant and helper first landed between a comment and the constant it describes, leaving the NFT_TIMEOUT_MS note sitting above MAX_TIMEOUT_MS. Moved so nothing is orphaned. Every claim in those comments is measured, including the one above about truncation, and 2147483648 warns and fires at once.

Provenance and scope

This is my own defect: I wrote the history clamp in #93 and #97 copied the construction — comment included — onto the NFT path. One fix belongs in both places because there is one source, and the tests showed that plainly: the history fallback was pinned with six shapes and the NFT side with exactly one. Infinity was among the six, but with a fixture that answers at once that row could not tell the default from the cap; the NFT table did not list it at all. Both tables are the same set now, and the comment beside the NFT one says why they are kept identical — an asymmetric pair is how the next copy loses coverage again.

I had reproduced that asymmetry in this branch before noticing it: my first version widened the history table and left the NFT one a shape short.

Not reachable through the CLI: cli.mjs:416 calls getHistory() with no arguments, and the NFT read reaches the wallet from src/tools.mjs:972 as wallet.getNfts(owner) — an owner, no options. What is affected is timeoutMs as a documented option of two exported functions, and a comment that promised more than the code delivered.

[portdeveloper]

lgtm, thanks