aboutsummaryrefslogtreecommitdiffstats
path: root/packages/meshbay-hub/tests/harness
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-07 16:02:10 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-07 16:02:10 +0200
commitd1f998b42137465b610667439527917a00030b4d (patch)
tree13674bc358ce585b0a2cd14ac210486bed79ff29 /packages/meshbay-hub/tests/harness
parent8883d60d0afa2ed9dd1ef68bc21fe1b9a65a59ff (diff)
downloadmeshbay-d1f998b42137465b610667439527917a00030b4d.tar.gz
fix(mnp): give a reply an id, so it stops being routed by luck
MNP carried no correlation id. A reply named its own type and nothing else, so a client with more than one request in flight worked out which one a message answered from the message itself — and for the replies that name nothing it could not. `_dispatch` fell through to matching by arrival order, which is a guess. `_sendAndWait` had the right value all along: it keys `_pending` by `this._seqId++` and never put it on the wire. The guess fails asymmetrically, which is why it hid. The victim is not the request that was answered wrongly — it is the unrelated one that now waits out its own 30s timeout for a reply already delivered elsewhere. Live on 2026-09-06: five `music_meta_req` sat pending for over 100 seconds behind a failing MusicBrainz, and a `device_list_result` was handed to one of them. The composer is disabled while a send is in flight, so a chat message whose reply went astray the same way left the Chat tab looking frozen for thirty seconds, then unfroze on its own. The `ack` half of this was fixed on 2026-08-30 by matching on request type. That closed the instance and left the class open: a refusal has no type to match on either, and `_dispatch_message`'s catch-all answers every unforeseen failure with `{"type": "error", "detail": "Request failed"}` — 238 of this module's 240 error sends name nothing at all. `req_id` now rides on the request and comes back on the reply. On the node it is published for the whole handler in a ContextVar and stamped by `_send`: a parameter would have meant threading an argument through all 240 send sites, and asyncio copies the context into a task, so a handler that `_spawn`s its real work still answers under the right id. It is never stamped on a broadcast — those answer nothing, and the owner check in `_send` is what keeps a chat broadcast or an index push from reaching another peer looking like a reply. On the client, `_dispatch` resolves on `req_id` first and the arrival-order fallback is gone the moment a node proves it stamps (`_correlates`, armed by the handshake's own reply). The fallback stays for an MNP 1.0 node, unchanged and no wider: there it is the only thing there is, and removing it would leave device_list_result, join_result and the handshake replies reaching nobody. Two things fall out. `sendChat` refuses an `error` reply like every other request in the file — it returned it as success, which did not matter while a refusal reached the wrong caller anyway and would now show a rejected message as sent. And `_group_ctx` uses `.get`: a reload pops a removed group while sessions connected to it are open, and every request they had left raised KeyError into that same catch-all. Sealed index messages are the one exception to the fast path. They cannot be handed over until they are opened, which is asynchronous while `_dispatch` is not — resolving on the id alone gave `fetchIndex` the envelope and skipped `onIndexSync` entirely. Caught by extending `index_seal_probe.mjs` to stamp a reply the way a current node does, after the hub suite passed over it: the probe built its own frames and had never seen one. Tests, all failing before and passing after: `test_chat_send.py` drives the real ChatPanel over the real transport for both shapes of reply with an older request pending (3 of its 6 are new, and the 3 for `ack` pass either way, so it discriminates); `test_reply_correlation.py` pins the node's half — the refusals that name nothing else, the broadcast that must not be stamped, and a late reply from a spawned task answering under its own id rather than the most recent request's. Full suite: 1897 passed, same 11 pre-existing failures as before. QUIC keeps its own dispatch and is not stamped. It is disabled by default and no browser request reaches it, but the asymmetry is real. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dn1xYx9uT69mCB6UDvyKAN
Diffstat (limited to 'packages/meshbay-hub/tests/harness')
-rw-r--r--packages/meshbay-hub/tests/harness/chat_send_probe.py145
1 files changed, 89 insertions, 56 deletions
diff --git a/packages/meshbay-hub/tests/harness/chat_send_probe.py b/packages/meshbay-hub/tests/harness/chat_send_probe.py
index f1cc191..98983f3 100644
--- a/packages/meshbay-hub/tests/harness/chat_send_probe.py
+++ b/packages/meshbay-hub/tests/harness/chat_send_probe.py
@@ -20,8 +20,25 @@ never appeared, while the node had stored it all along.
chat_send_probe.py
-Prints JSON: `steps`, the state of the panel at each stage, and `log`, what the
-transport sent and how the deliberately-unanswered request ended up.
+It now drives two shapes of reply, because there were two ways for one to go
+astray and only the first was ever fixed:
+
+ `ack` the node accepts the message and answers `{"type": "ack"}`, which
+ names no request. Routed by request type since 2026-08-30.
+ `error` the node *refuses* it — every failure in `_dispatch_message` ends at
+ one catch-all sending `{"type": "error", "detail": "Request failed"}`,
+ and 238 of this module's 240 error sends name nothing either. That
+ reply reached no caller at all: it went to whatever request happened
+ to be waiting, and the send sat out its own 30s timeout with the
+ composer disabled.
+
+Both are run with an older request already pending — the condition that turns
+"guess by arrival order" from usually-right into wrong — and both must come
+back inside a second and a half.
+
+Prints JSON: `scenarios`, the state of the panel at each stage of each, and
+`log`, what the transport sent and how the deliberately-unanswered request
+ended up.
"""
import http.server
import json
@@ -54,13 +71,6 @@ import { ChatPanel } from '/chat-app.js';
const log = [];
window.addEventListener('error', e => log.push('error: ' + e.message));
-// The real transport, with only the channel replaced: _send takes the plain
-// object _sendAndWait built, so the framing and msgpack are the only things
-// skipped — every pending entry, key and dispatch path below is the shipped one.
-const tp = new window.MeshBayTransport('', 'token');
-tp._connected = true;
-tp._channel = { readyState: 'open', send() {} };
-
const now = Date.now() / 1000;
const history = [];
for (let i = 0; i < 5; i++) {
@@ -68,50 +78,45 @@ for (let i = 0; i < 5; i++) {
payload: 'message ' + i, timestamp: now - (5 - i) * 60 });
}
-// Stands in for the node, answering exactly what webrtc_server.py answers.
-// media_meta_req is answered with nothing at all, which is what a refusal
-// amounts to for the request that asked: `_do_media_meta_request` sends a bare
-// `error` for a file_id the index does not have, and a bare error names no
-// request, so it reaches none.
-tp._send = (obj) => {
- log.push('sent ' + obj.type);
- if (obj.type === 'chat_hist') {
- setTimeout(() => tp._dispatch(
- { type: 'chat_hist_resp', v: '0.2', messages: history, has_more: false }), 10);
- } else if (obj.type === 'chat_msg') {
- setTimeout(() => tp._dispatch({ type: 'ack', v: '0.14' }), 10);
- }
-};
+const wait = ms => new Promise(r => setTimeout(r, ms));
-function Host() {
+// Stands in for the node, answering what webrtc_server.py answers — including
+// stamping the reply with the id of the request it is answering, which is what
+// `_send` does there. `chatReply` is the only difference between the two runs.
+function makeTransport(chatReply) {
+ // The real transport, with only the channel replaced: _send takes the plain
+ // object _sendAndWait built, so the framing and msgpack are the only things
+ // skipped — every pending entry, key and dispatch path below is the shipped
+ // one.
+ const tp = new window.MeshBayTransport('', 'token');
+ tp._connected = true;
+ tp._channel = { readyState: 'open', send() {} };
+ tp._send = (obj) => {
+ log.push('sent ' + obj.type);
+ const answer = (reply) => setTimeout(
+ () => tp._dispatch({ ...reply, req_id: obj.req_id }), 10);
+ if (obj.type === 'chat_hist') {
+ answer({ type: 'chat_hist_resp', v: '0.2', messages: history, has_more: false });
+ } else if (obj.type === 'chat_msg') {
+ answer(chatReply);
+ }
+ // music_meta_req is answered by nothing at all, on purpose: the node holds
+ // one open for as long as the third-party lookup behind it takes, which
+ // was measured at over 100 seconds with that service failing. It is the
+ // older pending request every scenario here needs.
+ };
+ return tp;
+}
+
+function Host({ tp }) {
const transportRef = useRef(tp);
const gekRef = useRef(null);
return html`<${ChatPanel} transportRef=${transportRef} gekRef=${gekRef}
username="me" entries=${[]} status="connected" />`;
}
-render(html`<${Host} />`, document.getElementById('root'));
-
-const out = { steps: [], log };
-const composer = () => document.querySelector('.chat-input');
-
-function snap(label) {
- const c = composer();
- out.steps.push({
- label,
- bubbles: document.querySelectorAll('.chat-bubble').length,
- lastText: [...document.querySelectorAll('.chat-text')].pop()?.textContent ?? null,
- // What a frozen tab actually is: the composer is disabled for as long as
- // a send is in flight.
- composerDisabled: c ? c.disabled : null,
- composerValue: c ? c.value : null,
- pending: tp._pending.size,
- });
-}
-
-const wait = ms => new Promise(r => setTimeout(r, ms));
-function typeInto(text) {
- const c = composer();
+function typeInto(root, text) {
+ const c = root.querySelector('.chat-input');
c.focus();
// Preact reads e.target.value on input, so the native setter has to run.
Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, 'value')
@@ -119,22 +124,42 @@ function typeInto(text) {
c.dispatchEvent(new Event('input', { bubbles: true }));
}
-(async () => {
+async function runScenario(name, chatReply) {
+ const root = document.createElement('div');
+ document.getElementById('root').appendChild(root);
+ const tp = makeTransport(chatReply);
+ render(html`<${Host} tp=${tp} />`, root);
+
+ const steps = [];
+ const snap = (label) => {
+ const c = root.querySelector('.chat-input');
+ steps.push({
+ label,
+ bubbles: root.querySelectorAll('.chat-bubble').length,
+ lastText: [...root.querySelectorAll('.chat-text')].pop()?.textContent ?? null,
+ // What a frozen tab actually is: the composer is disabled for as long as
+ // a send is in flight.
+ composerDisabled: c ? c.disabled : null,
+ composerValue: c ? c.value : null,
+ pending: tp._pending.size,
+ });
+ };
+
await wait(500);
snap('arrived');
- // The Videos tab asked about a file a moment ago and is still waiting. Any
- // unanswered request will do; this is the one that was live when the defect
- // was found.
- tp.fetchMediaMeta('a-file-the-node-refused')
- .then(m => log.push('media_meta resolved with ' + m.type),
- e => log.push('media_meta rejected: ' + e.message));
+ // The Music tab asked about a track and is still waiting on the node, which
+ // is waiting on something else. Any older unanswered request will do; this
+ // is the one that was live when the defect was found.
+ tp.fetchMusicMeta('a-track-the-node-is-slow-about')
+ .then(m => log.push(name + ': music_meta resolved with ' + m.type),
+ e => log.push(name + ': music_meta rejected: ' + e.message));
await wait(100);
- snap('stale request pending');
+ snap('older request pending');
- typeInto('hello');
+ typeInto(root, 'hello');
await wait(100);
- composer().dispatchEvent(new KeyboardEvent('keydown',
+ root.querySelector('.chat-input').dispatchEvent(new KeyboardEvent('keydown',
{ key: 'Enter', bubbles: true, cancelable: true }));
// Far short of _sendAndWait's 30s timeout: a send that has not come back by
@@ -142,6 +167,14 @@ function typeInto(text) {
await wait(1500);
snap('after send');
+ return { name, steps };
+}
+
+(async () => {
+ const out = { scenarios: [], log };
+ out.scenarios.push(await runScenario('ack', { type: 'ack', v: '0.14' }));
+ out.scenarios.push(await runScenario(
+ 'error', { type: 'error', detail: 'Request failed' }));
fetch('/log', { method: 'POST', body: JSON.stringify(out) });
})();
</script></body></html>"""