diff options
Diffstat (limited to 'packages/meshbay-hub')
7 files changed, 239 insertions, 18 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js index fb9d801..65860ec 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js @@ -94,7 +94,15 @@ function FilesPanel({ for (const file of files) { transfers.start({ kind: 'upload', name: file.name, total: file.size, transport, - lease: transport.openTransfer({ kind: 'upload', bytes: file.size }), + // `makeLease`, not `lease`: pausing gives the slot back, so resuming + // has to be able to ask for another one, and a transfer handed a lease + // it cannot re-create is refused the button rather than offered one + // that would drop its slot for good. + makeLease: () => transport.openTransfer({ kind: 'upload', + bytes: file.size }), + // A `File` is seekable and the node remembers how much it holds, so + // there is no target tier to consult here — unlike a download. + pausable: true, run: async ({ signal, onProgress, lease }) => { await transport.uploadFile(file, { // Bytes the node acknowledged, not bytes read locally. diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js index 797321c..524b7f3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js @@ -141,7 +141,7 @@ export class TransferStore { * there is somewhere to write — see file-utils.js's downloadEntry. */ start({ kind, name, total = 0, transport = null, run, open = null, - lease = null, prepare = null, makeLease = null }) { + lease = null, prepare = null, makeLease = null, pausable = false }) { const item = { id: _nextId++, kind, name, total, transport, open, lease, @@ -156,8 +156,11 @@ export class TransferStore { error: '', samples: [{ t: this._now(), done: 0 }], signal: { aborted: false, paused: false }, - // Set from `prepare`: whether this target can be stopped and continued. - pausable: false, + // Whether this transfer can be stopped and continued. A download learns + // it from `prepare`, because only its target knows; an upload says so + // outright, because a `File` is always seekable and the node keeps the + // position (see uploads.py). + pausable: Boolean(pausable), // Where a resumed run picks up, in chunks. Zero until something pauses. resumeFrom: 0, // Resolved by resume(); awaited by the run loop while paused. diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index 54a1302..23823ac 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -40,6 +40,16 @@ async function _pkEdFromSk(skPkcs8B64) { // flight, which saturates any path up to roughly 100 Mb/s at 100 ms. const UPLOAD_CHUNK_SIZE = 48 * 1024; const UPLOAD_WINDOW = 32; +// "Where am I?", asked as an ordinary sealed upload chunk with no bytes rather +// than on a clear message. Mirrors UPLOAD_PROBE_INDEX in +// meshbay_common/protocol.py; the node writes nothing and answers with +// `resume_from`, and one that predates it refuses the index, which reads as +// "start from the beginning". +const UPLOAD_PROBE_INDEX = -1; +// How long to wait for that answer before assuming there is none. A node that +// answers neither the probe nor its refusal must not leave an upload waiting +// for ever, and starting over is always safe. +const UPLOAD_PROBE_TIMEOUT_MS = 5000; const UPLOAD_BUFFER_HIGH = 1024 * 1024; // Segments of 256 KB: 24 in flight is 6 MB, enough to keep playback fed over a @@ -2409,8 +2419,23 @@ class MeshBayTransport { const waiter = acks.shift(); if (waiter) waiter(); }; + // "Where am I?" — resolved by the node's answer to the probe chunk below, + // or by anything that says this node cannot answer it. + let settleProbe = null; + const probed = new Promise((r) => { settleProbe = r; }); + const answerProbe = (from) => { + if (!settleProbe) return false; + const done = settleProbe; + settleProbe = null; + done(from); + return true; + }; this._uploaders.set(uploadId, (msg) => { if (msg.type === 'error') { + // A node that predates the probe refuses its index. That is not a + // failure — it is the answer "start from the beginning", which is what + // this client did before there was anything to ask. + if (answerProbe(0)) return; failure = new Error(msg.detail || 'Upload refused'); wake(); return; @@ -2423,19 +2448,75 @@ class MeshBayTransport { .then((plain) => { const payload = msgpack_decode(plain); if (payload.stored_as) stored = payload; + // Only the probe's answer carries this, so the two are told apart + // without trusting the index the node echoed back in clear. + if (typeof payload.resume_from === 'number') return answerProbe(payload.resume_from); + return false; }) .catch((e) => { failure = new Error( `The node's upload reply did not open under the group key (${e.message})`); + return false; }) - .finally(wake); + // A probe's answer is not a chunk: waking here would credit the + // progress bar with a chunk that was never sent. + .then((wasProbe) => { if (!wasProbe) wake(); }); }); const nextAck = () => new Promise(r => acks.push(r)); try { - for (let i = 0; i < total; i++) { + // Ask before sending anything. An upload interrupted at 99% used to start + // again from zero, because the node kept its position on the connection + // that was lost — see `uploads.py`. The question goes inside the seal, as + // a chunk with no bytes, because naming the file on a clear message is + // exactly what sealing this path was for. + // Sealed first, spread second — the same shape as the chunk loop below, + // and not only for symmetry: `test_the_upload_itself_is_sealed` reads + // this call and fails if a filename appears in it, which is how it can + // tell a field outside the seal from one inside it. + const probeSealed = await C.sealGroup( + this._gekRaw, 'upload', 'file_upload', groupId, + msgpack_encode({ filename: file.name, data: new Uint8Array(0), + dir: dir || '', root: root || '' })); + this._send({ + type: 'file_upload', + v: '0.1', + upload_id: uploadId, + chunk_index: UPLOAD_PROBE_INDEX, + total_chunks: total, + ...(tr ? { tr } : {}), + ...probeSealed, + }); + // Bounded: a node that answers neither the probe nor its refusal must not + // leave an upload waiting for ever. Starting over is always safe. + let from = await Promise.race([ + probed, + new Promise((r) => setTimeout(() => { answerProbe(0); r(0); }, + UPLOAD_PROBE_TIMEOUT_MS)), + ]); + // Defensive: a node reporting a position at or past the end would have + // renamed the file and dropped its state, so this cannot happen — and if + // it does, sending everything again is the answer that cannot corrupt. + if (!(from > 0) || from >= total) from = 0; + if (from > 0) { + acked = from; + if (onProgress) onProgress(Math.min(file.size, from * size), file.size); + } + + for (let i = from; i < total; i++) { if (signal && signal.aborted) throw _aborted(); + // Between two chunks, never inside one — the node refuses a chunk that + // is not the one it expects, so a position is the only thing worth + // remembering. Nothing is recorded here beyond that: the node holds the + // real position, and the probe above is what asks for it on the way + // back in, which makes resuming correct even across a reconnect. + if (signal && signal.paused) { + signal.resumeFrom = i; + const paused = new Error('Paused'); + paused.name = 'PausedError'; + throw paused; + } // Backpressure: without it the whole file lands in the browser's send // buffer in seconds and the progress bar becomes a work of fiction. while (this._channel && this._channel.bufferedAmount > UPLOAD_BUFFER_HIGH) { diff --git a/packages/meshbay-hub/tests/harness/upload_seal_probe.mjs b/packages/meshbay-hub/tests/harness/upload_seal_probe.mjs index 0b77e42..a6008c2 100644 --- a/packages/meshbay-hub/tests/harness/upload_seal_probe.mjs +++ b/packages/meshbay-hub/tests/harness/upload_seal_probe.mjs @@ -72,13 +72,34 @@ tp._nodeVersion = input.node_version; const frames = []; let uploadId = null; +let answered = 0; tp._send = (msg) => { frames.push(toHex(msgpack_encode(msg))); if (msg.upload_id) uploadId = msg.upload_id; - if (input.mode !== 'receive') return; + if (input.mode !== 'receive') { + // Nothing answers in this mode -- except the probe, which the client waits + // five seconds for. A node that predates it refuses the index, and that + // refusal is a plain error rather than a sealed ack, so the harness can + // produce it honestly. It is also the degradation path worth exercising. + if (msg.chunk_index === -1) { + // With `probe_ack`, answer it the way a node holding part of this file + // does; without, the way one that predates the probe does. + const reply = input.probe_ack + ? Object.assign(msgpack_decode(hex(input.probe_ack)), + { upload_id: uploadId }) + : { type: 'error', upload_id: uploadId, + code: 'bad_chunk_index', detail: 'Unexpected chunk index' }; + setImmediate(() => tp._dispatch(reply)); + } + return; + } // Answer as the node did, on the next turn of the loop so the send path // finishes first — which is also how a real ack arrives. - const ack = msgpack_decode(hex(input.acks[msg.chunk_index])); + // + // By position, not by `chunk_index`: the node answers every frame including + // the probe, whose index is -1, and the two lists are built from the same + // sequence of frames. + const ack = msgpack_decode(hex(input.acks[answered++])); ack.upload_id = uploadId; // Through the real `_dispatch`, so the routing under test — matching an // ack to its uploader by `upload_id` — is the shipped one. diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index 035f569..f27d4fd 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -713,3 +713,60 @@ def test_a_paused_transfer_still_counts_as_live(tmp_path): say('pending:' + t.pending); """, tmp_path) assert out == ["pending:1"] + + +def test_an_upload_can_be_paused_without_a_prepare_step(tmp_path): + """A download learns whether it can pause from its target, because only the + target knows. An upload has no target to ask: a `File` is seekable and the + node keeps the position, so it says so outright. + + This was missed when pause shipped — the button appeared on downloads and + nowhere else, including in the desktop app where everything else works. + """ + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const leases = []; + t.start({ + kind: 'upload', name: 'f', total: 100, pausable: true, + makeLease: () => { const l = new L(); leases.push(l); return l; }, + run: async ({ signal, from }) => { + for (let i = from || 0; i < 10; i++) { + await new Promise(r => setTimeout(r, 5)); + if (signal.paused) { + signal.resumeFrom = i; + const e = new Error('p'); e.name = 'PausedError'; throw e; + } + } + }, + }); + await new Promise(r => setTimeout(r, 5)); + leases[0].grant(); + await new Promise(r => setTimeout(r, 20)); + say('pausable:' + t.list()[0].pausable); + t.pause(t.list()[0].id); + await new Promise(r => setTimeout(r, 30)); + say('status:' + t.list()[0].status); + say('released:' + leases[0].released.join(',')); + """, tmp_path) + assert out == ["pausable:true", "status:paused", "released:paused"] + + +def test_an_upload_handed_a_lease_it_cannot_recreate_is_not_offered_pause(tmp_path): + """Pausing gives the slot back. A transfer that cannot ask for another one + would pause once and wait for ever, so the button is refused instead.""" + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + t.start({ kind: 'upload', name: 'f', total: 100, pausable: true, lease, + run: async ({ signal }) => { + while (!signal.aborted) await new Promise(r => setTimeout(r, 5)); + } }); + lease.grant(); + await new Promise(r => setTimeout(r, 20)); + const id = t.list()[0].id; + t.pause(id); + await new Promise(r => setTimeout(r, 20)); + say('status:' + t.list()[0].status); + t.cancel(id); + """, tmp_path) + assert out == ["status:running"] diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py index 879062b..fe550f9 100644 --- a/packages/meshbay-hub/tests/test_transport_contracts.py +++ b/packages/meshbay-hub/tests/test_transport_contracts.py @@ -349,16 +349,29 @@ def test_the_upload_itself_is_sealed(transport): "the upload must be sealed under the group key") assert "openGroup(" in body and "'file_upload_ack'" in body, ( "the ack carries the stored name and must be opened, not read") - # The message the node actually receives: everything between `this._send({` - # and its close. Read on its own, because the same field names appear a few - # lines above inside `msgpack_encode({...})`, which is the sealed half. - sent = body[body.index("this._send({"):] - sent = sent[:sent.index("});")] - assert "filename" not in sent, "the filename is on the message in clear" - assert "data" not in sent, "the bytes are on the message in clear" - assert "dir" not in sent and "root" not in sent, ( - "the destination is on the message in clear") - assert "...sealed," in sent, "the message must carry the sealed pair" + # The messages the node actually receives: everything between each + # `this._send({` and its close. Read on their own, because the same field + # names appear a few lines above inside `msgpack_encode({...})`, which is + # the sealed half. + # + # Every one of them, not the first: `uploadFile` sends a probe chunk before + # the file ("where am I?", UPLOAD_PROBE_INDEX) and it names the file too, so + # a check that stopped at the first message would have moved off the one it + # was written for the day the second appeared. + sends = [] + rest = body + while "this._send({" in rest: + rest = rest[rest.index("this._send({"):] + sends.append(rest[:rest.index("});")]) + rest = rest[len("this._send({"):] + assert len(sends) >= 2, "the probe and the chunks are both sent from here" + for sent in sends: + assert "filename" not in sent, "the filename is on the message in clear" + assert "data" not in sent, "the bytes are on the message in clear" + assert "dir" not in sent and "root" not in sent, ( + "the destination is on the message in clear") + assert "...sealed," in sent or "...probeSealed," in sent, ( + "the message must carry the sealed pair") assert "supportsSealedUpload" in body, ( "an older node must be refused before a chunk is sent, not after") diff --git a/packages/meshbay-hub/tests/test_upload_seal_client.py b/packages/meshbay-hub/tests/test_upload_seal_client.py index d6f9156..2e4bfb5 100644 --- a/packages/meshbay-hub/tests/test_upload_seal_client.py +++ b/packages/meshbay-hub/tests/test_upload_seal_client.py @@ -167,3 +167,41 @@ def test_the_client_refuses_an_older_node_before_sending_a_chunk(_gek): assert result["state"] == "rejected" assert "older MeshBay" in result["message"] assert result["frames"] == [], "a chunk was sent to a node that cannot open it" + + +def test_an_interrupted_upload_resumes_where_the_node_stopped(tmp_path, _gek): + """ + The browser asks, the node answers, and the second attempt sends only what + is missing. + + Both halves are the shipped ones: the frames come from the real + `uploadFile`, the answer comes from the real node handler. What is asserted + is the thing that used to be impossible — an upload interrupted at chunk two + of five that sends three chunks instead of five. + """ + body = bytes(range(256)) * ((CHUNK * 5) // 256 + 1) + body = body[:CHUNK * 5] + first = _run_probe(_probe_input(_gek, "send", + file={"name": "film.mkv", "data": body.hex()})) + frames = [msgpack.unpackb(bytes.fromhex(f), raw=False) + for f in first["frames"]] + assert [f["chunk_index"] for f in frames] == [-1, 0, 1, 2, 3, 4] + + # The link drops after two chunks. + session = _node_session(tmp_path, _gek) + for frame in frames[1:3]: + session._do_file_upload(frame) + assert not [m for m in session.sent if m.get("type") == "error"] + + # It comes back and asks. + session.sent.clear() + session._do_file_upload(frames[0]) + probe_ack = msgpack.packb(session.sent[-1], use_bin_type=True).hex() + + second = _run_probe(_probe_input( + _gek, "send", file={"name": "film.mkv", "data": body.hex()}, + probe_ack=probe_ack)) + resumed = [msgpack.unpackb(bytes.fromhex(f), raw=False)["chunk_index"] + for f in second["frames"]] + assert resumed == [-1, 2, 3, 4], ( + f"sent {resumed} — the answer to the probe was not used") |