feat(browser): a boot failure is a state with a reason, not silence

improvise-1wft and the five beads it blocked were one failure: nothing
carried a boot failure's REASON from the worker to the page. The channel
is one rule -- a protocol frame crosses the port as a STRING, a shell
notice as an OBJECT -- so it needs no protocol change and the
discriminator is total.

A boot failure is a STATE, not a rejected promise. A document another
context holds the exclusive handle to now OPENS read-only: measured first
(harness probe, Chromium + Firefox), getFile() reads the bytes while the
handle is held, with the second-handle refusal asserted so the probe
cannot pass vacuously.

- improvise-1wft  the channel: Notice/NoticeKind, boot_notice, take_notice
- improvise-i0v9  read-only boot over vfs::ReadOnly + OpfsFs::read_unattached
- improvise-86nw  start() rejecting for any reason now posts BootFailed
- improvise-9cma  boot::document_name -- and a/b/c.improv was ACCEPTED as
                  c.improv, not rejected, which is worse than the report
- improvise-iobc  vfs::Origin: a seed error is a notice only when the
                  document really had to be created without it
- improvise-vep8  the watchdog arms BEFORE transport.ready; its message
                  stopped claiming the connection was accepted
- improvise-7gip  DedicatedWorkerGlobalScope (needs a web-sys feature)
- improvise-zre1  a persist that fails raises a notice; the trigger is the
                  dirty flag surviving, observed rather than predicted
- improvise-7gf8  the render breaker trips once instead of retrying forever
- improvise-ociy  non-OPEN sends are loud; a viewport announcement is not
- improvise-y01k  the three ws-server drops are logged
- improvise-r4nf  serde failures throw instead of reading as "nothing to send"

Every notice routes through shell.fail, so gotcha 48's disarm half is not
re-broken. Two comments in shell.spec.js stated the opposite of the new
arming order and were corrected rather than left.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-21 14:51:34 -07:00
parent 4f194e9f26
commit 12714c7527
25 changed files with 1225 additions and 84 deletions
Generated
+1
View File
@@ -965,6 +965,7 @@ dependencies = [
"improvise-core", "improvise-core",
"improvise-io", "improvise-io",
"improvise-protocol", "improvise-protocol",
"serde",
"serde_json", "serde_json",
"wasm-bindgen", "wasm-bindgen",
"wasm-bindgen-futures", "wasm-bindgen-futures",
+234
View File
@@ -2574,3 +2574,237 @@ The lockstep half of the entry above: it asserts the host itself answers
view). Without it, "the client settles to X" is a claim about the client alone — view). Without it, "the client settles to X" is a claim about the client alone —
a host that answered `Normal` everywhere would make the switch test's a host that answered `Normal` everywhere would make the switch test's
expectations arbitrary rather than lockstep. expectations arbitrary rather than lockstep.
## The browser boot diagnosis channel (improvise-1wft and the five it blocked)
Five beads filed by three reviewers were one failure: **nothing carried a boot
failure's REASON from the worker to the page.** `web/worker.js` caught its own
`start()` rejection and logged it, a worker that rejects in `start()` fires no
`error` on the main thread (gotcha 46), and `onFrame`'s `if (!server) return`
dropped every frame after that — so the only survivor was a generic 8s
watchdog, which improvise-vep8 showed did not even arm on one path.
The channel is one rule: **a protocol frame crosses the port as a STRING and a
shell notice crosses as an OBJECT.** Nothing else crosses, so the discriminator
is total and costs no protocol change — `ServerFrame` JSON is a string by
construction of `WorkerServer::on_message`. `shell.js::classifyWorkerMessage`
is that rule and `web/tests/shell.test.js` pins it, including the case that
would break it: a notice *serialized* would classify as a frame and be fed to
the fold.
**A boot failure is a STATE, not a rejected promise.** That is what improvise-i0v9
needs: the page must open and paint read-only when another context holds the
document's exclusive handle. So `bindings.rs::boot` matches on `attach`'s
failure rather than propagating it — a locked document falls through to
`read_only`, which reads the bytes the non-exclusive way and opens over a
backend that refuses every write. `BootFailed` is the only fatal kind;
`ReadOnly`, `SeedMissing` and `SaveFailed` are all open, painting sessions,
which is what `shell.js::isFatal` says and what its test asserts in both
directions.
### The `getFile()` probe (`harness/`, not a cargo test)
**The measurement the read-only path rests on, taken before it was promised.**
`Storage::read` goes through the exclusive sync access handle, so the read-only
session needed a path that does not: `FileSystemFileHandle.getFile()` yields a
`File` snapshot, which is a `Blob`, which reads without a handle. Whether that
works *while another context holds the handle* was unprobed, and the whole
design turns on it, so it was measured rather than assumed.
Measured 2026-09-21, `harness/tests/opfs.spec.js`, both engines that have OPFS:
| engine | `getFile()` + `arrayBuffer()` under a foreign sync handle | the refusal that proves the lock |
|---|---|---|
| Chromium | **32 bytes read, correct** | `NoModificationAllowedError: Failed to execute 'createSyncAccessHandle' on 'FileSystemFileHandle': Access Handles cannot be created if there is another open Access Handle or Writable stream associated with the same file.` |
| Firefox | **32 bytes read, correct** | `NoModificationAllowedError: No modification allowed` |
| WebKit | not measured — Playwright's WebKit provides no OPFS at all | — |
**The precondition is the half that makes it evidence.** Without it the probe
passes vacuously in any engine that does not actually enforce exclusivity: the
read would succeed because there is no lock, not because `getFile()` bypasses
one. So the probe first tries to take a *second* `createSyncAccessHandle` and
`ensure!`s it is **refused**, and reports the refusal text alongside the byte
count. Both halves are in the attached `opfs-probes.json`.
This is filed as a probe rather than a contract check on purpose: it records
what the engines do, so a decision taken on it can be re-checked rather than
re-argued. It runs per engine, so a regression in either is a failure.
### the_exclusive_handle_refusal_is_recognised_in_both_engines
`boot::is_locked` decides whether an `attach` failure means "somebody else has
it" (open read-only) or "something else is wrong" (fail). It matches on
`NoModificationAllowedError`, which is a **string match on a DOMException
name**, and the two engines word the rest of the message completely
differently — so the fixtures are the strings the harness probe above actually
produced, not invented ones. That is the only reason the match is trustworthy.
The discriminating half is the negative: a `SecurityError` from `getDirectory()`
must **not** classify as a lock, or every boot failure would offer a read-only
session over a document it never read. An empty rendering is not a lock either.
### a_read_only_backend_reads_the_document_and_refuses_every_write
`vfs::ReadOnly` is what makes improvise-i0v9's "open and paint" true rather than
aspirational. The test asserts both directions, which matters because a wrapper
that forwarded everything would pass a reads-only assertion: the document opens
and reads back, `write` and `delete` are both refused, the refusal **carries the
reason** rather than a bare error, and the refused writes left the readable
bytes intact.
`delete` is refused as well as `write`, which is easy to miss — it is a write.
### a_document_created_without_its_promised_seed_says_it_was_created
improvise-iobc: a bad or missing `?seed=` produced an empty document and a
normal-looking shell, with the 404 only in the console. The page cannot tell an
*intended* empty document from one that is empty because its starting copy
never arrived — and neither could anything else, because `seedBytes()` returned
`undefined` for both.
The fix is not in the page: `web/worker.js` now returns `{bytes}` or `{error}`
and hands **both** to Rust, and `vfs::open` reports an `Origin` — `Stored`,
`Seeded` or `Created`. Only `Created` **with** a seed error is a notice, so a
reload of an existing document says nothing, which is the common case. That
conjunction is the point: either fact alone over-reports.
The test's discriminating half is that the *same call* reports `Seeded` when a
seed is supplied, so `Created` is not a constant.
### a_name_that_is_not_one_flat_entry_is_refused_rather_than_truncated
improvise-9cma reported `?doc=` values like `` and `///` giving a blank UI and
generic watchdog text. Probing it turned up something worse than the reported
symptom: `OpfsFs::entry_name` takes `Path::file_name`, so **`?doc=a/b/c.improv`
was not rejected at all — it silently opened `c.improv`**, a different document
from the one the URL named. An accepted-and-wrong is worse than a refusal,
because the diagnosis looks identical and the repair is not.
`boot::document_name` answers before anything touches storage, so the reason is
specific and arrives early rather than as a late failure deep in the handle
acquisition.
### a_persist_that_fails_raises_a_notice_naming_the_document
improvise-zre1: the worker auto-persists on its own behalf, and a failed write
left its message in `view_state.status_msg` — which the worker has no UI for —
while `dirty` stayed true. The client saw `dirty: true` and could not tell
"save failed" from "save pending", which is a data-loss risk on page close if
the user trusts autosave.
**The trigger is an observation, not a prediction**, in the same shape gotcha 45
gives for the persist trigger itself: a save that worked clears the flag, so
*the flag surviving `persist()` is* the failure. No second table of which
effects can fail.
The fault is injected with `vfs::ReadOnly` — a real backend that reads and
refuses writes — rather than a mock, and the test asserts the precondition
(`dirty` really did survive) before believing the notice. The notice is
**drained**, not read, so one occurrence is reported once; asserting the second
`take_notice()` is `None` is what pins that.
Its companion `a_persist_that_succeeds_raises_no_notice` is not padding: without
it the first test passes against a worker that reports a failure on every single
persist.
### a connection that hangs forever is still caught by the watchdog
improvise-vep8: `shell.arm` ran **after** `await transport.ready`, so a socket
that never fires `open` never armed the watchdog at all — the page sat on
"connecting…" forever with no timeout. Arming now precedes the await.
Two consequences worth recording, because both were invisible until the move.
**The message had to change.** It said `accepted the connection but sent no
document`, which arming before `ready` makes a claim the page no longer knows
to be true. It now says `never answered`, which covers a peer that never opened
*and* one that opened and went quiet — the one condition the watchdog is really
for, and the reason gotcha 46 gives for it not being per-transport. The existing
spec asserted the old wording and failed, which is the right way round.
**Two comments in `shell.spec.js` stated the opposite of the new code** — both
said `shell.arm` runs only after `transport.ready` and that the watchdog was
"not on this path". Corrected in the same change rather than left, per gotcha
48's third clause: when a fact is stated in more than one place, changing it is
a grep and not an edit.
The disarm half of gotcha 48 is **not** re-broken, and this is the thing to
check first on any future change here: every notice goes through `shell.fail`,
which disarms, so a precise reason is never replaced 8s later. `a socket that
opened and then dropped reads as disconnected, and stays that way` still passes
and still runs past the window.
### an invalid ?doc= is named on screen, and not blamed on the browser
The boundary test for the channel: a `?doc=` Rust refuses produces a
`BootFailed` notice carrying the refusal text, the page renders it, and it
**stands past the watchdog window**. The negative assertion uses
`outlastTheWatchdog`, which asks `timesArmed() > 0` first — mandatory here,
since denying the watchdog's message on a path where it never armed denies
nothing.
### a seed that 404s says so rather than showing a silent empty grid
The other half, and it asserts the thing that separates it from a boot failure:
the page **still draws a grid**. An unfetchable seed is a degraded session, not
a dead one, so `#grid table` must be visible *and* `#status` must say `EMPTY`
and name the seed that failed.
### The silent-drop shape, grepped rather than taken one site at a time
`ociy`, `y01k` and `r4nf` are all "an error swallowed into a value that reads as
'nothing to do'", so the workspace was grepped for the shape rather than only
the sites the tickets named.
- **`serde_json::(to_string|from_str)…\.ok()`** across `crates/` and `src/`:
three instances, all in `improvise-wasm-client/src/lib.rs`, all named by
improvise-r4nf. **Zero remain** after the fix, which is the check that the
grep and the ticket agreed. They now return `Result<Option<String>, JsError>`
— the `Option` still means "nothing to send", and serialization failure
throws, so JS's `if (frame)` is unchanged at every call site.
- **Discarded results in the two hosts**: the remaining `let _ = …` in
`transport.rs` are channel sends whose receiver being gone *is* the normal
disconnect path, and are correctly ignored. The three improvise-y01k named
are now loud.
- **`postMessage` with no guard** in `web/app.js`'s worker transport: not a
second instance of improvise-ociy. `postMessage` on a live dedicated worker
does not throw and the page never calls `terminate()`; the analogous silent
drop is worker-side (`if (!server) return`), and that is improvise-86nw,
answered by the `BootFailed` notice rather than by a send guard.
One thing the send guard had to be split over: a `ResizeObserver` firing after a
drop would have overwritten the precise `disconnected` with `not sent`, breaking
the property gotcha 48 says keeps getting lost. **A viewport announcement is not
a user edit**, so it sends `{ loud: false }` and only user intent reports a
failed send.
### The render circuit breaker (improvise-7gf8)
`repaint()` returned early when `render_model()` threw, leaving `painted`
unchanged — so the `model.revision === painted` guard could never short-circuit
and every subsequent event retried the same broken render forever. It is now
tripped once: a render model that cannot be built is a permanent state, so the
breaker is a flag rather than a counter, and the message says to reload, which
is the only recovery the page has. It routes through `shell.fail`, so it also
disarms the watchdog rather than being replaced by it.
### `OpfsFs::open`'s scope guard (improvise-7gip)
`dyn_into::<WorkerGlobalScope>()` is satisfied by a `SharedWorkerGlobalScope`,
so a shared worker sailed past a check whose message named the right
requirement and failed later as a raw `TypeError` in the glue.
`DedicatedWorkerGlobalScope` is the type that actually means what the message
says.
**The fix needed a `web-sys` feature, which is why it had not been made
cheaply**: `DedicatedWorkerGlobalScope` is feature-gated and absent by default,
so the one-line change does not compile until
`crates/improvise-io/Cargo.toml` names it. That is the whole cost, and it is
worth stating because "just use the narrower type" reads as free.
`a_dedicated_worker_satisfies_the_narrow_scope_guard` is the guard that the fix
did not break the shipped path: in a real dedicated worker the narrow cast
succeeds. It is deliberately **not** a proof that a SharedWorker is rejected —
that needs a SharedWorker page, which this harness does not have, and
improvise-12ha owns the multi-tab question. Recorded as a gap rather than
implied.
+1
View File
@@ -30,6 +30,7 @@ wasm-bindgen-futures = "0.4"
version = "0.3" version = "0.3"
features = [ features = [
"Blob", "Blob",
"DedicatedWorkerGlobalScope",
"File", "File",
"FileSystemDirectoryHandle", "FileSystemDirectoryHandle",
"FileSystemFileHandle", "FileSystemFileHandle",
+22 -1
View File
@@ -60,8 +60,10 @@ impl OpfsFs {
/// startup. /// startup.
pub async fn open() -> Result<Self> { pub async fn open() -> Result<Self> {
let global = js_sys::global(); let global = js_sys::global();
// NOT `WorkerGlobalScope`, which a SharedWorker satisfies while lacking
// `createSyncAccessHandle` entirely (improvise-7gip, improvise-12ha).
let storage = global let storage = global
.dyn_into::<web_sys::WorkerGlobalScope>() .dyn_into::<web_sys::DedicatedWorkerGlobalScope>()
.map_err(|_| anyhow!("OPFS sync access handles require a dedicated Web Worker"))? .map_err(|_| anyhow!("OPFS sync access handles require a dedicated Web Worker"))?
.navigator() .navigator()
.storage(); .storage();
@@ -101,6 +103,25 @@ impl OpfsFs {
Ok(()) Ok(())
} }
/// Read one document WITHOUT the exclusive handle, through `getFile()`.
/// Measured to work under a foreign handle: `context/code/clients.md`.
pub async fn read_unattached(&self, path: &Path) -> Result<Vec<u8>> {
let name = entry_name(path)?;
let file = self
.file_handle(&name)
.await
.ok_or_else(|| anyhow!("{name} is not a file in this origin"))?;
let blob = JsFuture::from(file.get_file())
.await
.map_err(|e| js_err(&format!("getFile({name})"), e))?
.dyn_into::<web_sys::File>()
.map_err(|_| anyhow!("getFile({name}) did not yield a File"))?;
let buffer = JsFuture::from(blob.array_buffer())
.await
.map_err(|e| js_err(&format!("arrayBuffer({name})"), e))?;
Ok(js_sys::Uint8Array::new(&buffer).to_vec())
}
/// Close and forget one document's handle, releasing the exclusive /// Close and forget one document's handle, releasing the exclusive
/// lock so another context can open it. /// lock so another context can open it.
pub fn detach(&self, path: &Path) -> Result<()> { pub fn detach(&self, path: &Path) -> Result<()> {
+46 -16
View File
@@ -25,6 +25,12 @@ pub fn init() -> Client {
Client::default() Client::default()
} }
/// One frame as the JSON that crosses the wire. Separate so a serialization
/// failure cannot be spelled `.ok()` and read as "nothing to send" (r4nf).
fn frame_json(frame: &ClientFrame) -> Result<String, serde_json::Error> {
serde_json::to_string(frame)
}
#[wasm_bindgen] #[wasm_bindgen]
impl Client { impl Client {
#[wasm_bindgen(constructor)] #[wasm_bindgen(constructor)]
@@ -42,7 +48,7 @@ impl Client {
alt: bool, alt: bool,
shift: bool, shift: bool,
meta: bool, meta: bool,
) -> Option<String> { ) -> Result<Option<String>, JsError> {
let browser = BrowserKey { let browser = BrowserKey {
key: key.to_string(), key: key.to_string(),
ctrl, ctrl,
@@ -50,8 +56,10 @@ impl Client {
shift, shift,
meta, meta,
}; };
let message = self.key(&browser)?; self.key(&browser)
serde_json::to_string(&message).ok() .map(|f| frame_json(&f))
.transpose()
.map_err(|e| JsError::new(&e.to_string()))
} }
/// Has anything moved? The cheap question, and the one `on_frame` already /// Has anything moved? The cheap question, and the one `on_frame` already
@@ -93,40 +101,48 @@ impl Client {
/// The frame for "the user typed `text` into the cell at this viewport /// The frame for "the user typed `text` into the cell at this viewport
/// position", or `None` — which is a race, not a normal path. /// position", or `None` — which is a race, not a normal path.
#[wasm_bindgen] #[wasm_bindgen]
pub fn typed_cell(&self, row: usize, col: usize, text: &str) -> Option<String> { pub fn typed_cell(
serde_json::to_string(&self.typed_cell_frame(row, col, text)?).ok() &self,
row: usize,
col: usize,
text: &str,
) -> Result<Option<String>, JsError> {
self.typed_cell_frame(row, col, text)
.map(|f| frame_json(&f))
.transpose()
.map_err(|e| JsError::new(&e.to_string()))
} }
/// The user clicked row `index` of the category panel. Resolved whole — /// The user clicked row `index` of the category panel. Resolved whole —
/// focus, cursor and activation — and folded here (improvise-7ybz). /// focus, cursor and activation — and folded here (improvise-7ybz).
#[wasm_bindgen] #[wasm_bindgen]
pub fn click_category(&mut self, index: usize) -> Option<String> { pub fn click_category(&mut self, index: usize) -> Result<Option<String>, JsError> {
self.clicked(Click::CategoryRow(index)) self.clicked(Click::CategoryRow(index))
} }
/// The user clicked row `index` of the view panel: focus, move, switch. /// The user clicked row `index` of the view panel: focus, move, switch.
#[wasm_bindgen] #[wasm_bindgen]
pub fn click_view(&mut self, index: usize) -> Option<String> { pub fn click_view(&mut self, index: usize) -> Result<Option<String>, JsError> {
self.clicked(Click::ViewRow(index)) self.clicked(Click::ViewRow(index))
} }
/// The user clicked row `index` of the formula panel. Navigation only — /// The user clicked row `index` of the formula panel. Navigation only —
/// that panel binds no `Enter` (improvise-iioi). /// that panel binds no `Enter` (improvise-iioi).
#[wasm_bindgen] #[wasm_bindgen]
pub fn click_formula(&mut self, index: usize) -> Option<String> { pub fn click_formula(&mut self, index: usize) -> Result<Option<String>, JsError> {
self.clicked(Click::FormulaRow(index)) self.clicked(Click::FormulaRow(index))
} }
/// The user clicked tile `index`: select it and cycle its axis. /// The user clicked tile `index`: select it and cycle its axis.
#[wasm_bindgen] #[wasm_bindgen]
pub fn click_tile(&mut self, index: usize) -> Option<String> { pub fn click_tile(&mut self, index: usize) -> Result<Option<String>, JsError> {
self.clicked(Click::Tile(index)) self.clicked(Click::Tile(index))
} }
/// The user clicked a grid cell at a **viewport** position: move the /// The user clicked a grid cell at a **viewport** position: move the
/// selection there. Opening an editor is the page's own affair. /// selection there. Opening an editor is the page's own affair.
#[wasm_bindgen] #[wasm_bindgen]
pub fn click_cell(&mut self, row: usize, col: usize) -> Option<String> { pub fn click_cell(&mut self, row: usize, col: usize) -> Result<Option<String>, JsError> {
self.clicked(Click::Cell { row, col }) self.clicked(Click::Cell { row, col })
} }
} }
@@ -174,8 +190,11 @@ impl Client {
Some(ClientFrame::Commands(cmds)) Some(ClientFrame::Commands(cmds))
} }
fn clicked(&mut self, target: Click) -> Option<String> { fn clicked(&mut self, target: Click) -> Result<Option<String>, JsError> {
serde_json::to_string(&self.click_frame(target)?).ok() self.click_frame(target)
.map(|f| frame_json(&f))
.transpose()
.map_err(|e| JsError::new(&e.to_string()))
} }
/// Classify a keypress, folding it locally when this client can answer it. /// Classify a keypress, folding it locally when this client can answer it.
@@ -251,7 +270,10 @@ mod tests {
1 1
); );
let outbound = client.on_key_event("R", false, false, true, false).unwrap(); let outbound = client
.on_key_event("R", false, false, true, false)
.unwrap()
.unwrap();
assert_eq!( assert_eq!(
serde_json::from_str::<ClientFrame>(&outbound).unwrap(), serde_json::from_str::<ClientFrame>(&outbound).unwrap(),
ClientFrame::command(Command::SetBuffer { ClientFrame::command(Command::SetBuffer {
@@ -279,6 +301,7 @@ mod tests {
let before = client.revision(); let before = client.revision();
let outbound = client let outbound = client
.on_key_event("Enter", false, false, false, false) .on_key_event("Enter", false, false, false, false)
.unwrap()
.unwrap(); .unwrap();
assert!( assert!(
outbound.contains("Key"), outbound.contains("Key"),
@@ -297,7 +320,9 @@ mod tests {
fn a_cursor_key_before_the_handshake_produces_no_message() { fn a_cursor_key_before_the_handshake_produces_no_message() {
let mut client = init(); let mut client = init();
assert_eq!( assert_eq!(
client.on_key_event("ArrowDown", false, false, false, false), client
.on_key_event("ArrowDown", false, false, false, false)
.unwrap(),
None None
); );
} }
@@ -317,9 +342,14 @@ mod tests {
#[test] #[test]
fn a_browser_shortcut_produces_no_message_at_all() { fn a_browser_shortcut_produces_no_message_at_all() {
let mut client = init(); let mut client = init();
assert_eq!(client.on_key_event("r", false, false, false, true), None);
assert_eq!( assert_eq!(
client.on_key_event("Shift", false, false, true, false), client.on_key_event("r", false, false, false, true).unwrap(),
None
);
assert_eq!(
client
.on_key_event("Shift", false, false, true, false)
.unwrap(),
None None
); );
} }
@@ -137,6 +137,7 @@ fn a_browser_client_seeds_types_a_number_and_sees_it_come_back() {
); );
let frame = client let frame = client
.typed_cell(at.data_row, at.col, "7500") .typed_cell(at.data_row, at.col, "7500")
.expect("serialization must not fail")
.expect("an editable cell must compose a write"); .expect("an editable cell must compose a write");
let frames = send(&mut host, id, &frame); let frames = send(&mut host, id, &frame);
@@ -219,6 +220,7 @@ fn a_grouped_row_axis_does_not_shift_the_write() {
let frame = client let frame = client
.typed_cell(at.data_row, at.col, "4200") .typed_cell(at.data_row, at.col, "4200")
.expect("serialization must not fail")
.expect("an editable cell must compose a write"); .expect("an editable cell must compose a write");
send(&mut host, id, &frame); send(&mut host, id, &frame);
@@ -285,7 +287,7 @@ fn an_aggregated_cell_reaches_the_page_as_not_editable() {
"every cell here aggregates over Product, so none may be written" "every cell here aggregates over Product, so none may be written"
); );
assert_eq!( assert_eq!(
client.typed_cell(0, 0, "42"), client.typed_cell(0, 0, "42").unwrap(),
None, None,
"and typing into one composes nothing, because the id is of the wrong kind" "and typing into one composes nothing, because the id is of the wrong kind"
); );
@@ -334,12 +336,16 @@ fn a_commit_keypress_is_one_mutation_among_view_commands_and_is_accepted() {
// Type into the cell under the cursor the way the page does: enter edit // Type into the cell under the cursor the way the page does: enter edit
// mode, type, commit. // mode, type, commit.
for key in ["i", "8", "8", "8"] { for key in ["i", "8", "8", "8"] {
if let Some(frame) = client.on_key_event(key, false, false, false, false) { if let Some(frame) = client
.on_key_event(key, false, false, false, false)
.unwrap()
{
send(&mut host, id, &frame); send(&mut host, id, &frame);
} }
} }
let commit = client let commit = client
.on_key_event("Enter", false, false, false, false) .on_key_event("Enter", false, false, false, false)
.expect("serialization must not fail")
.expect("a commit must compose a frame"); .expect("a commit must compose a frame");
// The shape under test: several commands, exactly one a mutation — // The shape under test: several commands, exactly one a mutation —
@@ -16,6 +16,7 @@ improvise-io = { path = "../improvise-io" }
improvise-protocol = { path = "../improvise-protocol" } improvise-protocol = { path = "../improvise-protocol" }
improvise-command = { path = "../improvise-command" } improvise-command = { path = "../improvise-command" }
anyhow = "1" anyhow = "1"
serde = { version = "1", features = ["derive"] }
serde_json = "1" serde_json = "1"
# The browser half. Only the wasm build pulls these in, so `cargo test` on the # The browser half. Only the wasm build pulls these in, so `cargo test` on the
+106 -13
View File
@@ -7,16 +7,21 @@ use std::rc::Rc;
use anyhow::Result; use anyhow::Result;
use improvise_command::ui::app::App; use improvise_command::ui::app::App;
use improvise_io::opfs::OpfsFs; use improvise_io::opfs::OpfsFs;
use improvise_io::storage::{MemoryFs, Storage};
use wasm_bindgen::prelude::*; use wasm_bindgen::prelude::*;
use crate::vfs::{self, Opening}; use crate::boot::{Notice, NoticeKind, document_name, is_locked};
use crate::vfs::{self, Opening, Origin, ReadOnly};
use crate::worker::Worker; use crate::worker::Worker;
/// The worker's handle for the lifetime of the page. /// The worker's handle for the lifetime of the page.
#[wasm_bindgen] #[wasm_bindgen]
pub struct WorkerServer { pub struct WorkerServer {
worker: Worker, worker: Worker,
storage: Rc<OpfsFs>, /// `None` in a read-only session: there is no handle to flush through.
storage: Option<Rc<OpfsFs>>,
/// What the page must say about this boot, if anything.
notice: Option<Notice>,
} }
/// The `JsError` half of [`crate::report`]; the rendering lives in `lib.rs` /// The `JsError` half of [`crate::report`]; the rendering lives in `lib.rs`
/// because this module is `cfg(target_arch = "wasm32")` and cannot be tested. /// because this module is `cfg(target_arch = "wasm32")` and cannot be tested.
@@ -27,30 +32,92 @@ fn report(e: anyhow::Error) -> JsError {
/// Boot the worker over OPFS and open `document`. The document is named **out /// Boot the worker over OPFS and open `document`. The document is named **out
/// of band**, not in a frame — see `context/code/clients.md`. /// of band**, not in a frame — see `context/code/clients.md`.
#[wasm_bindgen] #[wasm_bindgen]
pub async fn start(document: &str, seed: Option<Vec<u8>>) -> Result<WorkerServer, JsError> { pub async fn start(
boot(document, seed.as_deref()).await.map_err(report) document: &str,
seed: Option<Vec<u8>>,
seed_error: Option<String>,
) -> Result<WorkerServer, JsError> {
boot(document, seed.as_deref(), seed_error.as_deref())
.await
.map_err(report)
} }
async fn boot(document: &str, seed: Option<&[u8]>) -> Result<WorkerServer> { async fn boot(
document: &str,
seed: Option<&[u8]>,
seed_error: Option<&str>,
) -> Result<WorkerServer> {
// A name this backend cannot hold is a reason of its own, answered before
// anything touches storage (improvise-9cma).
let path = document_name(document).map_err(|why| anyhow::anyhow!("{why}"))?;
let storage = Rc::new(OpfsFs::open().await?); let storage = Rc::new(OpfsFs::open().await?);
// `list` answers from this, and `vfs::open` asks it whether to seed. // `list` answers from this, and `vfs::open` asks it whether to seed.
storage.refresh().await?; storage.refresh().await?;
let path = PathBuf::from(document);
// Acquiring a handle is the async half; reading and writing through it are // Acquiring a handle is the async half; reading and writing through it are
// not. Nothing may touch this document before the await completes. // not. Nothing may touch this document before the await completes.
storage.attach(&path).await?; match storage.attach(&path).await {
Ok(()) => writable(storage, path, seed, seed_error),
// improvise-i0v9: the handle is somebody else's. The bytes are still
// readable, so the session OPENS rather than dying silently.
Err(e) if is_locked(&crate::report(&e)) => {
read_only(&storage, path, crate::report(&e)).await
}
Err(e) => Err(e),
}
}
/// The ordinary boot: this context holds the handle and may write.
fn writable(
storage: Rc<OpfsFs>,
path: PathBuf,
seed: Option<&[u8]>,
seed_error: Option<&str>,
) -> Result<WorkerServer> {
let opening = match seed { let opening = match seed {
Some(bytes) => Opening::named(path.clone()).with_seed(bytes), Some(bytes) => Opening::named(path.clone()).with_seed(bytes),
None => Opening::named(path.clone()), None => Opening::named(path.clone()),
}; };
let workbook = vfs::open(&*storage, &opening)?; let opened = vfs::open(&*storage, &opening)?;
// An empty document the user did not ask for, because the seed it was
// promised never arrived (improvise-iobc).
let notice = match (opened.origin, seed_error) {
(Origin::Created, Some(why)) => Some(Notice::new(
NoticeKind::SeedMissing,
path.display().to_string(),
why,
)),
_ => None,
};
let mut app = App::with_storage(workbook, Some(path), Box::new(Rc::clone(&storage))); let mut app = App::with_storage(opened.workbook, Some(path), Box::new(Rc::clone(&storage)));
app.rebuild_layout(); app.rebuild_layout();
Ok(WorkerServer { Ok(WorkerServer {
worker: Worker::new(app), worker: Worker::new(app),
storage, storage: Some(storage),
notice,
})
}
/// Another context holds the handle. Read the bytes the non-exclusive way and
/// open over a backend that refuses every write — `context/code/clients.md`.
async fn read_only(storage: &OpfsFs, path: PathBuf, why: String) -> Result<WorkerServer> {
let bytes = storage.read_unattached(&path).await?;
let mirror = MemoryFs::new();
mirror.write(&path, &bytes)?;
let fs = ReadOnly::new(mirror, "another tab or window is holding this document");
let opened = vfs::open(&fs, &Opening::named(path.clone()))?;
let mut app = App::with_storage(opened.workbook, Some(path.clone()), Box::new(fs));
app.rebuild_layout();
Ok(WorkerServer {
worker: Worker::new(app),
storage: None,
notice: Some(Notice::new(
NoticeKind::ReadOnly,
path.display().to_string(),
why,
)),
}) })
} }
@@ -63,18 +130,41 @@ impl WorkerServer {
self.worker.on_message(json).map_err(report) self.worker.on_message(json).map_err(report)
} }
/// What the page must say about this boot, as JSON, or `None`. Read once,
/// straight after `start` — see `context/code/clients.md`.
#[wasm_bindgen]
pub fn boot_notice(&self) -> Result<Option<String>, JsError> {
self.notice
.as_ref()
.map(|n| serde_json::to_string(n).map_err(|e| JsError::new(&e.to_string())))
.transpose()
}
/// A notice the worker raised while running, as JSON, or `None`. The one
/// that exists is a failed auto-persist (improvise-zre1).
#[wasm_bindgen]
pub fn take_notice(&mut self) -> Result<Option<String>, JsError> {
self.worker
.take_notice()
.map(|n| serde_json::to_string(&n).map_err(|e| JsError::new(&e.to_string())))
.transpose()
}
/// Flush the removals `Storage::delete` queued: visible to `read`/`list` /// Flush the removals `Storage::delete` queued: visible to `read`/`list`
/// at once, landing in OPFS here. The JS loop awaits it after each message. /// at once, landing in OPFS here. The JS loop awaits it after each message.
#[wasm_bindgen] #[wasm_bindgen]
pub async fn flush(&self) -> Result<(), JsError> { pub async fn flush(&self) -> Result<(), JsError> {
self.storage.commit_deletes().await.map_err(report) match &self.storage {
Some(fs) => fs.commit_deletes().await.map_err(report),
None => Ok(()),
}
} }
/// Every document in the origin's storage, for a picker on the main thread: /// Every document in the origin's storage, for a picker on the main thread:
/// `Storage::list`, which is why a browser needed `list` at all. /// `Storage::list`, which is why a browser needed `list` at all.
#[wasm_bindgen] #[wasm_bindgen]
pub fn documents(&self) -> Result<Vec<String>, JsError> { pub fn documents(&self) -> Result<Vec<String>, JsError> {
vfs::documents(&*self.storage) vfs::documents(self.worker.app().storage.as_ref())
.map(|paths| { .map(|paths| {
paths paths
.iter() .iter()
@@ -87,6 +177,9 @@ impl WorkerServer {
/// Re-read the directory listing `documents` answers from. /// Re-read the directory listing `documents` answers from.
#[wasm_bindgen] #[wasm_bindgen]
pub async fn refresh(&self) -> Result<(), JsError> { pub async fn refresh(&self) -> Result<(), JsError> {
self.storage.refresh().await.map_err(report) match &self.storage {
Some(fs) => fs.refresh().await.map_err(report),
None => Ok(()),
}
} }
} }
+127
View File
@@ -0,0 +1,127 @@
//! What can go wrong before the first frame, as data the page can render.
//! Why a boot failure is a STATE and not a rejection: `context/code/clients.md`.
use std::path::{Path, PathBuf};
use serde::{Deserialize, Serialize};
/// Something the page must say out loud. It crosses `postMessage` as an
/// OBJECT, where a protocol frame crosses as a string (`context/code/clients.md`).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Notice {
pub kind: NoticeKind,
/// The document this is about, so a message can name it.
pub document: String,
/// The underlying cause, already rendered. May be empty.
pub detail: String,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum NoticeKind {
/// Another context holds the exclusive handle. The document is READABLE
/// and the session is open; nothing can be saved.
ReadOnly,
/// Boot did not produce a session at all. Nothing works.
BootFailed,
/// The document had to be created and the seed it was promised never
/// arrived, so what is on screen is empty for a reason worth saying.
SeedMissing,
/// A write the user did not ask for, and did not get.
SaveFailed,
}
impl Notice {
pub fn new(kind: NoticeKind, document: impl Into<String>, detail: impl Into<String>) -> Self {
Self {
kind,
document: document.into(),
detail: detail.into(),
}
}
}
/// Whether a rendered OPFS error is the exclusive-handle refusal. Measured
/// wording for both engines is in `context/test-evidence/clients.md`.
pub fn is_locked(rendered: &str) -> bool {
rendered.contains("NoModificationAllowedError")
}
/// What `?doc=` may name: one entry in the flat OPFS root. `OpfsFs` takes the
/// `file_name` of whatever it is given, so anything else was silently accepted.
pub fn document_name(raw: &str) -> Result<PathBuf, String> {
let trimmed = raw.trim();
let path = Path::new(trimmed);
let named = path.file_name().and_then(|n| n.to_str()).unwrap_or("");
// The whole value must BE the name. A path with anything around it was
// being silently truncated to its last component (improvise-9cma).
let flat = named == trimmed && !trimmed.is_empty();
if flat {
Ok(PathBuf::from(named))
} else {
Err(format!(
"{raw:?} does not name a document: use one plain file name, such as \
`notes.improv`, with no `/` and no `..`"
))
}
}
#[cfg(test)]
mod tests {
use super::*;
/// The exact strings both engines produce, captured by the harness probe
/// rather than guessed. Evidence: `context/test-evidence/clients.md`.
#[test]
fn the_exclusive_handle_refusal_is_recognised_in_both_engines() {
let chromium = "createSyncAccessHandle(two-tab.improv): JsValue(NoModificationAllowedError: \
Failed to execute 'createSyncAccessHandle' on 'FileSystemFileHandle': Access \
Handles cannot be created if there is another open Access Handle or Writable \
stream associated with the same file.)";
let firefox = "createSyncAccessHandle(two-tab.improv): JsValue(NoModificationAllowedError: \
No modification allowed\n)";
assert!(is_locked(chromium), "chromium's wording went unrecognised");
assert!(is_locked(firefox), "firefox's wording went unrecognised");
// The discriminating half: an unrelated failure must NOT be read as a
// lock, or every boot failure would offer a read-only session.
assert!(
!is_locked("getDirectory(): JsValue(SecurityError: The operation is insecure.)"),
"a non-lock error was classified as a lock"
);
assert!(!is_locked(""), "an empty rendering is not a lock");
}
#[test]
fn a_document_name_is_one_flat_entry() {
assert_eq!(
document_name("notes.improv").unwrap(),
PathBuf::from("notes.improv")
);
// Surrounding whitespace is a typo, not a different document.
assert_eq!(
document_name(" notes.improv ").unwrap(),
PathBuf::from("notes.improv")
);
}
/// Each of these USED to open something: `entry_name` takes `file_name`,
/// so `a/b/c.improv` silently opened `c.improv` (improvise-9cma).
#[test]
fn a_name_that_is_not_one_flat_entry_is_refused_rather_than_truncated() {
for raw in ["", " ", "///", "a/b/c.improv", "../escape.improv", "dir/"] {
let err =
document_name(raw).expect_err(&format!("{raw:?} was accepted as a document name"));
assert!(
err.contains("does not name a document"),
"the refusal must say what is wrong: {err}"
);
}
// The one that matters most, spelled out: it was not rejected before,
// it was ACCEPTED as a different document than the user named.
assert!(
document_name("a/b/c.improv").is_err(),
"a path was accepted and silently truncated to its last component"
);
}
}
+3 -1
View File
@@ -7,14 +7,16 @@ pub fn report(e: &anyhow::Error) -> String {
format!("{e:#}") format!("{e:#}")
} }
pub mod boot;
pub mod vfs; pub mod vfs;
pub mod worker; pub mod worker;
#[cfg(target_arch = "wasm32")] #[cfg(target_arch = "wasm32")]
pub mod bindings; pub mod bindings;
pub use boot::{Notice, NoticeKind};
pub use improvise_protocol::{ClientFrame, ServerFrame}; pub use improvise_protocol::{ClientFrame, ServerFrame};
pub use vfs::{Opening, documents}; pub use vfs::{Opened, Opening, Origin, documents};
pub use worker::Worker; pub use worker::Worker;
#[cfg(test)] #[cfg(test)]
+146 -7
View File
@@ -43,14 +43,85 @@ impl<'a> Opening<'a> {
} }
} }
/// Where the bytes a session opened on came from. `Created` is the arm the
/// page must hear about when a seed was promised (improvise-iobc).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Origin {
/// The backend already held it; any seed was ignored.
Stored,
/// Written from the seed just now.
Seeded,
/// Neither: a new empty model under that name.
Created,
}
/// A workbook and how it came to be. The second half is why this is not just
/// a `Workbook` — see [`Origin::Created`].
#[derive(Debug)]
pub struct Opened {
pub workbook: Workbook,
pub origin: Origin,
}
/// Open `opening.name`, creating it from the seed (or as an empty model) if the /// Open `opening.name`, creating it from the seed (or as an empty model) if the
/// backend lacks it. Always ends in a `read`: one path from bytes to `Workbook`. /// backend lacks it. Always ends in a `read`: one path from bytes to `Workbook`.
pub fn open(storage: &dyn Storage, opening: &Opening<'_>) -> Result<Workbook> { pub fn open(storage: &dyn Storage, opening: &Opening<'_>) -> Result<Opened> {
let path = opening.name.as_path(); let path = opening.name.as_path();
if !holds_a_document(storage, path)? { let origin = match holds_a_document(storage, path)? {
create(storage, opening)?; true => Origin::Stored,
false => {
create(storage, opening)?;
match opening.seed {
Some(_) => Origin::Seeded,
None => Origin::Created,
}
}
};
let workbook = persistence::load_from(storage, path)
.with_context(|| format!("Cannot open {}", path.display()))?;
Ok(Opened { workbook, origin })
}
/// A backend that answers reads and refuses every write, for a document another
/// context holds the exclusive handle to (improvise-i0v9). `context/code/clients.md`.
pub struct ReadOnly<S> {
inner: S,
why: String,
}
impl<S> ReadOnly<S> {
pub fn new(inner: S, why: impl Into<String>) -> Self {
Self {
inner,
why: why.into(),
}
}
}
impl<S: Storage> Storage for ReadOnly<S> {
fn read(&self, path: &Path) -> Result<Vec<u8>> {
self.inner.read(path)
}
fn write(&self, path: &Path, _bytes: &[u8]) -> Result<()> {
Err(anyhow::anyhow!(
"Cannot write {}: {}",
path.display(),
self.why
))
}
fn list(&self, dir: &Path) -> Result<Vec<PathBuf>> {
self.inner.list(dir)
}
fn delete(&self, path: &Path) -> Result<()> {
Err(anyhow::anyhow!(
"Cannot delete {}: {}",
path.display(),
self.why
))
} }
persistence::load_from(storage, path).with_context(|| format!("Cannot open {}", path.display()))
} }
/// Whether the backend holds a *document* under `path` — not merely an entry. /// Whether the backend holds a *document* under `path` — not merely an entry.
@@ -113,7 +184,7 @@ mod tests {
&Opening::named("demo.improv").with_seed(DEMO.as_bytes()), &Opening::named("demo.improv").with_seed(DEMO.as_bytes()),
) )
.expect("an empty entry is not a document, so the seed must still land"); .expect("an empty entry is not a document, so the seed must still land");
assert_eq!(wb.model.name, "Demo"); assert_eq!(wb.workbook.model.name, "Demo");
} }
#[test] #[test]
@@ -124,7 +195,8 @@ mod tests {
&Opening::named("demo.improv").with_seed(DEMO.as_bytes()), &Opening::named("demo.improv").with_seed(DEMO.as_bytes()),
) )
.expect("seeds and opens"); .expect("seeds and opens");
assert_eq!(wb.model.name, "Demo"); assert_eq!(wb.workbook.model.name, "Demo");
assert_eq!(wb.origin, Origin::Seeded);
assert_eq!( assert_eq!(
documents(&fs).unwrap(), documents(&fs).unwrap(),
vec![PathBuf::from("demo.improv")], vec![PathBuf::from("demo.improv")],
@@ -152,8 +224,14 @@ mod tests {
&Opening::named("demo.improv").with_seed(DEMO.as_bytes()), &Opening::named("demo.improv").with_seed(DEMO.as_bytes()),
) )
.expect("opens the stored copy"); .expect("opens the stored copy");
assert_eq!(
reopened.origin,
Origin::Stored,
"a document already there is not seeded, and must not say it was"
);
assert!( assert!(
reopened reopened
.workbook
.model .model
.category("Region") .category("Region")
.unwrap() .unwrap()
@@ -167,7 +245,8 @@ mod tests {
fn with_no_seed_an_absent_document_becomes_an_empty_model_that_is_saved() { fn with_no_seed_an_absent_document_becomes_an_empty_model_that_is_saved() {
let fs = MemoryFs::new(); let fs = MemoryFs::new();
let wb = open(&fs, &Opening::named("scratch.improv")).expect("creates"); let wb = open(&fs, &Opening::named("scratch.improv")).expect("creates");
assert_eq!(wb.model.name, "scratch"); assert_eq!(wb.workbook.model.name, "scratch");
assert_eq!(wb.origin, Origin::Created);
assert_eq!( assert_eq!(
documents(&fs).unwrap(), documents(&fs).unwrap(),
vec![PathBuf::from("scratch.improv")], vec![PathBuf::from("scratch.improv")],
@@ -206,4 +285,64 @@ mod tests {
"a document name's parent IS the root, or MemoryFs would filter it out" "a document name's parent IS the root, or MemoryFs would filter it out"
); );
} }
/// improvise-iobc: an empty document the user did not ask for is only
/// distinguishable from an intentional one by WHY it was created.
#[test]
fn a_document_created_without_its_promised_seed_says_it_was_created() {
let fs = MemoryFs::new();
// `seedBytes()` returned nothing, so no seed reaches `Opening`.
let opened = open(&fs, &Opening::named("probe.improv")).expect("creates");
assert_eq!(
opened.origin,
Origin::Created,
"an empty model made because the seed never arrived must be \
distinguishable from one made because none was asked for"
);
// The discriminating half: with a seed the SAME call reports Seeded,
// so this is not a constant.
let other = open(
&fs,
&Opening::named("seeded.improv").with_seed(DEMO.as_bytes()),
)
.expect("seeds");
assert_eq!(other.origin, Origin::Seeded);
}
/// improvise-i0v9: a document another context holds the handle to must
/// still OPEN. Reads pass through; every write is refused, loudly.
#[test]
fn a_read_only_backend_reads_the_document_and_refuses_every_write() {
let inner = MemoryFs::new();
inner
.write(Path::new("locked.improv"), DEMO.as_bytes())
.unwrap();
let fs = ReadOnly::new(inner, "another tab holds it");
let opened = open(&fs, &Opening::named("locked.improv"))
.expect("a locked document must still open read-only");
assert_eq!(opened.workbook.model.name, "Demo");
assert_eq!(opened.origin, Origin::Stored);
assert_eq!(
documents(&fs).unwrap(),
vec![PathBuf::from("locked.improv")]
);
let err = fs
.write(Path::new("locked.improv"), b"nope")
.expect_err("a write must not silently succeed");
assert!(
err.to_string().contains("another tab holds it"),
"the refusal must carry the reason: {err}"
);
assert!(
fs.delete(Path::new("locked.improv")).is_err(),
"delete is a write and must be refused too"
);
assert_eq!(
fs.read(Path::new("locked.improv")).unwrap(),
DEMO.as_bytes(),
"the refused writes must not have damaged what is readable"
);
}
} }
+114 -4
View File
@@ -7,12 +7,17 @@ use improvise_command::ui::host::{Addressed, Host};
use improvise_command::ui::project::SessionId; use improvise_command::ui::project::SessionId;
use improvise_protocol::{ClientFrame, ServerFrame}; use improvise_protocol::{ClientFrame, ServerFrame};
use crate::boot::{Notice, NoticeKind};
/// One document, one port, one client. Not `Debug`, for the reason `Host` is /// One document, one port, one client. Not `Debug`, for the reason `Host` is
/// not: it owns the whole document. /// not: it owns the whole document.
pub struct Worker { pub struct Worker {
host: Host, host: Host,
/// The main thread, once it has reported its extent. `None` until then. /// The main thread, once it has reported its extent. `None` until then.
client: Option<SessionId>, client: Option<SessionId>,
/// Raised by a persist nobody asked for and drained by the shim; see
/// [`Worker::take_notice`] and `context/code/clients.md`.
notice: Option<Notice>,
} }
impl Worker { impl Worker {
@@ -22,6 +27,7 @@ impl Worker {
Self { Self {
host: Host::new(app), host: Host::new(app),
client: None, client: None,
notice: None,
} }
} }
@@ -67,6 +73,28 @@ impl Worker {
self.host self.host
.app_mut() .app_mut()
.apply_effects(vec![Box::new(effect::Save)]); .apply_effects(vec![Box::new(effect::Save)]);
// A save that worked clears the flag, so the flag surviving IS the
// failure — observed, never predicted (improvise-zre1).
if self.host.app().model_state.dirty {
let app = self.host.app();
let document = app
.model_state
.file_path
.as_ref()
.map(|p| p.display().to_string())
.unwrap_or_default();
self.notice = Some(Notice::new(
NoticeKind::SaveFailed,
document,
app.view_state.status_msg.clone(),
));
}
}
/// Take whatever the last message raised. Drained rather than read, so a
/// notice is delivered once per occurrence.
pub fn take_notice(&mut self) -> Option<Notice> {
self.notice.take()
} }
/// The same, over the JSON that actually crosses a `MessagePort`. /// The same, over the JSON that actually crosses a `MessagePort`.
@@ -106,7 +134,7 @@ mod tests {
use improvise_core::model::cell::{CellKey, CellValue}; use improvise_core::model::cell::{CellKey, CellValue};
use improvise_core::view::Axis; use improvise_core::view::Axis;
use improvise_core::workbook::Workbook; use improvise_core::workbook::Workbook;
use improvise_io::storage::MemoryFs; use improvise_io::storage::{MemoryFs, Storage};
use improvise_protocol::{ use improvise_protocol::{
CellId, Command, GridCache, GridUpdate, KeyCode, KeyEvent, KeyModifiers, ModelCommand, CellId, Command, GridCache, GridUpdate, KeyCode, KeyEvent, KeyModifiers, ModelCommand,
Projection, RecordCell, WidgetCache, Projection, RecordCell, WidgetCache,
@@ -158,7 +186,9 @@ mod tests {
let fs = MemoryFs::new(); let fs = MemoryFs::new();
let path = PathBuf::from("demo.improv"); let path = PathBuf::from("demo.improv");
let seed = demo_bytes(); let seed = demo_bytes();
let wb = vfs::open(&fs, &Opening::named(&path).with_seed(&seed)).expect("the demo seeds"); let wb = vfs::open(&fs, &Opening::named(&path).with_seed(&seed))
.expect("the demo seeds")
.workbook;
let mut app = App::with_storage(wb, Some(path.clone()), Box::new(fs)); let mut app = App::with_storage(wb, Some(path.clone()), Box::new(fs));
app.rebuild_layout(); app.rebuild_layout();
(Worker::new(app), path) (Worker::new(app), path)
@@ -242,6 +272,86 @@ mod tests {
); );
} }
/// improvise-zre1: the failure stayed in a `status_msg` the worker has no
/// UI for, so the client could not tell "save failed" from "save pending".
#[test]
fn a_persist_that_fails_raises_a_notice_naming_the_document() {
let fs = MemoryFs::new();
let path = PathBuf::from("demo.improv");
fs.write(&path, &demo_bytes()).unwrap();
// The injected fault: a backend that reads and refuses every write.
let wb = vfs::open(&fs, &Opening::named(&path)).unwrap().workbook;
let refuses = crate::vfs::ReadOnly::new(fs, "the disk is full");
let mut app = App::with_storage(wb, Some(path.clone()), Box::new(refuses));
app.rebuild_layout();
let mut worker = Worker::new(app);
let mut grid = connect(&mut worker);
assert_eq!(
worker.take_notice(),
None,
"nothing has been written yet, so there is nothing to report"
);
let cell = addressed(&grid, 0, 0);
let cmd = reduce::caught_up(
worker.app(),
ModelCommand::SetCell {
cell,
value: CellValue::Number(42.0),
},
);
let out = worker.on_frame(ClientFrame::command(cmd));
let _ = folded(&mut grid, &out);
assert!(
worker.app().model_state.dirty,
"the precondition: the write really was refused, so the flag stands"
);
let notice = worker
.take_notice()
.expect("a failed auto-persist reached no one");
assert_eq!(notice.kind, NoticeKind::SaveFailed);
assert_eq!(notice.document, "demo.improv");
assert!(
!notice.detail.is_empty(),
"the notice must carry why, not merely that"
);
assert_eq!(
worker.take_notice(),
None,
"a notice is DRAINED, or the page would repeat one occurrence forever"
);
}
/// The other direction, without which the test above passes against a
/// worker that reports a failure on every single persist.
#[test]
fn a_persist_that_succeeds_raises_no_notice() {
let (mut worker, _) = worker_over_storage();
let mut grid = connect(&mut worker);
let cell = addressed(&grid, 0, 0);
let cmd = reduce::caught_up(
worker.app(),
ModelCommand::SetCell {
cell,
value: CellValue::Number(42.0),
},
);
let out = worker.on_frame(ClientFrame::command(cmd));
let _ = folded(&mut grid, &out);
assert!(
!worker.app().model_state.dirty,
"the precondition: this persist really did write"
);
assert_eq!(
worker.take_notice(),
None,
"a successful save must say nothing at all"
);
}
/// End to end over the VFS: open a seeded document, write a cell through /// End to end over the VFS: open a seeded document, write a cell through
/// the wire, and find the new value in what the client is projected. /// the wire, and find the new value in what the client is projected.
#[test] #[test]
@@ -286,7 +396,7 @@ mod tests {
let reopened = vfs::open(&*worker.app().storage, &Opening::named(&path)) let reopened = vfs::open(&*worker.app().storage, &Opening::named(&path))
.expect("the saved document reopens through the same VFS"); .expect("the saved document reopens through the same VFS");
assert_eq!( assert_eq!(
value_at(&reopened, 0, 0), value_at(&reopened.workbook, 0, 0),
Some(CellValue::Number(99.0)), Some(CellValue::Number(99.0)),
"the edit never reached the backend" "the edit never reached the backend"
); );
@@ -385,7 +495,7 @@ mod tests {
let reopened = vfs::open(&*worker.app().storage, &Opening::named(&path)) let reopened = vfs::open(&*worker.app().storage, &Opening::named(&path))
.expect("the document reopens through the same VFS"); .expect("the document reopens through the same VFS");
assert_eq!( assert_eq!(
reopened.active_view().category_axes, reopened.workbook.active_view().category_axes,
worker worker
.app() .app()
.model_state .model_state
@@ -59,7 +59,8 @@ impl MainThread {
fn boot_seeded(storage: MemoryFs, path: &Path, seed: &[u8]) -> Self { fn boot_seeded(storage: MemoryFs, path: &Path, seed: &[u8]) -> Self {
let workbook = vfs::open(&storage, &Opening::named(path).with_seed(seed)) let workbook = vfs::open(&storage, &Opening::named(path).with_seed(seed))
.expect("the demo seeds into an empty origin"); .expect("the demo seeds into an empty origin")
.workbook;
let mut app = App::with_storage(workbook, Some(path.to_path_buf()), Box::new(storage)); let mut app = App::with_storage(workbook, Some(path.to_path_buf()), Box::new(storage));
app.rebuild_layout(); app.rebuild_layout();
Self { Self {
@@ -213,7 +214,8 @@ fn a_reload_over_the_same_origin_reopens_what_was_saved() {
&*main.worker.app().storage, &*main.worker.app().storage,
&Opening::named(&path).with_seed(&demo_bytes()), &Opening::named(&path).with_seed(&demo_bytes()),
) )
.expect("reopens"); .expect("reopens")
.workbook;
let mut app = App::with_storage(workbook, Some(path.clone()), Box::new(MemoryFs::new())); let mut app = App::with_storage(workbook, Some(path.clone()), Box::new(MemoryFs::new()));
app.rebuild_layout(); app.rebuild_layout();
let mut reloaded = MainThread { let mut reloaded = MainThread {
+9 -1
View File
@@ -193,7 +193,15 @@ impl Registry {
pub fn handle(&mut self, who: &ClientId, msg: ClientFrame) -> Vec<Routed> { pub fn handle(&mut self, who: &ClientId, msg: ClientFrame) -> Vec<Routed> {
match self.documents.get_mut(&who.document) { match self.documents.get_mut(&who.document) {
Some(host) => Self::route(&who.document, host.handle(who.session, msg)), Some(host) => Self::route(&who.document, host.handle(who.session, msg)),
None => Vec::new(), // A document is never evicted while the server runs (ADR 001), so
// a `ClientId` always names one. Said out loud (improvise-y01k).
None => {
eprintln!(
"improvise: a frame arrived for {:?}, which this server is not holding",
who.document
);
Vec::new()
}
} }
} }
+12 -6
View File
@@ -179,8 +179,11 @@ async fn connection(
let json = match serde_json::to_string(&msg) { let json = match serde_json::to_string(&msg) {
Ok(json) => json, Ok(json) => json,
// Unreachable for the types involved, and not worth killing // Unreachable for the types involved, and not worth killing
// the connection over if it ever stops being. // the connection over. Loud, though (improvise-y01k).
Err(_) => continue, Err(err) => {
eprintln!("improvise: dropped a server frame it could not serialize: {err}");
continue;
}
}; };
if sink.send(Message::Text(json.into())).await.is_err() { if sink.send(Message::Text(json.into())).await.is_err() {
break; break;
@@ -239,11 +242,14 @@ where
{ {
while let Some(Ok(message)) = source.next().await { while let Some(Ok(message)) = source.next().await {
match message { match message {
Message::Text(text) => { Message::Text(text) => match serde_json::from_str::<ClientFrame>(&text) {
if let Ok(msg) = serde_json::from_str::<ClientFrame>(&text) { Ok(msg) => return Some(msg),
return Some(msg); // Not worth dropping the connection for, but not silent at
// both ends either (improvise-y01k).
Err(err) => {
eprintln!("improvise: ignored an unparseable client frame: {err}");
} }
} },
Message::Close(_) => return None, Message::Close(_) => return None,
_ => {} _ => {}
} }
+5
View File
@@ -32,9 +32,14 @@ wasm-bindgen-futures = "0.4"
[dependencies.web-sys] [dependencies.web-sys]
version = "0.3" version = "0.3"
features = [ features = [
"Blob",
"DedicatedWorkerGlobalScope",
"File",
"FileSystemDirectoryHandle", "FileSystemDirectoryHandle",
"FileSystemFileHandle",
"FileSystemGetDirectoryOptions", "FileSystemGetDirectoryOptions",
"FileSystemRemoveOptions", "FileSystemRemoveOptions",
"FileSystemSyncAccessHandle",
"Navigator", "Navigator",
"StorageManager", "StorageManager",
"WorkerGlobalScope", "WorkerGlobalScope",
+106
View File
@@ -292,6 +292,112 @@ pub async fn run_checks() -> Result<String, JsError> {
Ok(report.finish()) Ok(report.finish())
} }
/// Measurements the READ-ONLY boot design rests on (improvise-1wft/i0v9).
/// Not a contract; they record what the engine does. See clients.md.
#[wasm_bindgen]
pub async fn run_probes() -> Result<String, JsError> {
let mut report = Report::default();
let fs = OpfsFs::open()
.await
.map_err(|e| JsError::new(&format!("OpfsFs::open(): {e:#}")))?;
// The question improvise-i0v9's read-only path needs answered: can a
// context that CANNOT take the exclusive handle still read the bytes?
report
.run("getfile_reads_a_document_under_a_foreign_sync_handle", async || {
wipe(&fs).await?;
let p = Path::new("locked.improv");
fs.attach(p).await?;
fs.write(p, b"bytes behind an exclusive handle")?;
// The PRECONDITION, without which this probe passes vacuously:
// a second sync handle must really be refused.
let refusal = second_sync_handle("locked.improv").await;
ensure!(
refusal.is_some(),
"a second createSyncAccessHandle SUCCEEDED — this engine did not \
lock the file, so the read below measures nothing"
);
let bytes = read_via_get_file("locked.improv").await?;
ensure!(
bytes == b"bytes behind an exclusive handle",
"getFile() read back {:?}",
String::from_utf8_lossy(&bytes)
);
Ok(format!(
"getFile() read {} bytes while the handle was held; second handle refused with {}",
bytes.len(),
refusal.unwrap_or_default()
))
})
.await;
// The 7gip guard, from the inside: the scope this really is must satisfy
// the NARROW type, or the fix would break the shipped path.
report
.run("a_dedicated_worker_satisfies_the_narrow_scope_guard", async || {
let global = js_sys::global();
ensure!(
global
.clone()
.dyn_into::<web_sys::DedicatedWorkerGlobalScope>()
.is_ok(),
"a dedicated worker did not satisfy DedicatedWorkerGlobalScope"
);
ensure!(
global.dyn_into::<web_sys::WorkerGlobalScope>().is_ok(),
"a dedicated worker did not satisfy WorkerGlobalScope either"
);
Ok("both, as expected here — the SharedWorker page is where they differ".into())
})
.await;
Ok(report.finish())
}
/// Try to take a second exclusive handle on `name`. `Some(reason)` is a
/// refusal, which is what the probe above needs to be true.
async fn second_sync_handle(name: &str) -> Option<String> {
let root = opfs_root().await.ok()?;
let file = JsFuture::from(root.get_file_handle(name))
.await
.ok()?
.dyn_into::<web_sys::FileSystemFileHandle>()
.ok()?;
match JsFuture::from(file.create_sync_access_handle()).await {
Ok(handle) => {
// Do not leak the lock into the checks that follow.
if let Ok(h) = handle.dyn_into::<web_sys::FileSystemSyncAccessHandle>() {
h.close();
}
None
}
Err(e) => Some(format!("{e:?}")),
}
}
/// Read a whole entry through the NON-exclusive path: `getFile()` gives a
/// `File` snapshot, which is a `Blob`, which can be read without a handle.
async fn read_via_get_file(name: &str) -> Result<Vec<u8>> {
let root = opfs_root().await?;
let file = JsFuture::from(root.get_file_handle(name))
.await
.map_err(|e| anyhow!("getFileHandle({name}): {e:?}"))?
.dyn_into::<web_sys::FileSystemFileHandle>()
.map_err(|_| anyhow!("getFileHandle({name}) did not yield a file handle"))?;
let blob = JsFuture::from(file.get_file())
.await
.map_err(|e| anyhow!("getFile(): {e:?}"))?
.dyn_into::<web_sys::File>()
.map_err(|_| anyhow!("getFile() did not yield a File"))?;
let buffer = JsFuture::from(blob.array_buffer())
.await
.map_err(|e| anyhow!("arrayBuffer(): {e:?}"))?;
Ok(js_sys::Uint8Array::new(&buffer).to_vec())
}
/// Write the state the reload check expects, then hand control back. Split /// Write the state the reload check expects, then hand control back. Split
/// from [`after_reload`] because only a reload can ask whether OPFS persisted. /// from [`after_reload`] because only a reload can ask whether OPFS persisted.
#[wasm_bindgen] #[wasm_bindgen]
+20
View File
@@ -67,6 +67,26 @@ test.describe("OpfsFs in a real browser", () => {
).toEqual([]); ).toEqual([]);
}); });
// Measurements, not contracts: the read-only boot path (improvise-1wft,
// i0v9) is decided on these, so they run per engine and say which.
test("the probes the read-only boot design rests on", async ({ page }, testInfo) => {
const report = await run(page, "run_probes");
expect(report.map((c) => c.name)).toEqual([
"getfile_reads_a_document_under_a_foreign_sync_handle",
"a_dedicated_worker_satisfies_the_narrow_scope_guard",
]);
const failed = report.filter((c) => !c.ok);
expect(
failed,
`failing probes:\n${failed.map((c) => ` ${c.name}: ${c.detail}`).join("\n")}`,
).toEqual([]);
// The measurement is the point, so it is attached rather than only asserted.
await testInfo.attach("opfs-probes.json", {
body: JSON.stringify(report, null, 2),
contentType: "application/json",
});
});
// The one thing no in-page check can establish: "commit_deletes actually // The one thing no in-page check can establish: "commit_deletes actually
// removed it" is only answered by a fresh page. // removed it" is only answered by a fresh page.
test("what OPFS held survives a reload, and a committed delete does not", async ({ test("what OPFS held survives a reload, and a committed delete does not", async ({
+59 -9
View File
@@ -33,10 +33,10 @@ const json = (v) => JSON.stringify(v);
/// Walk past the watchdog without waiting for it. `page.clock` fakes the /// Walk past the watchdog without waiting for it. `page.clock` fakes the
/// page's own timers, so the 8s is the PAGE's and the wall clock is not. /// page's own timers, so the 8s is the PAGE's and the wall clock is not.
async function outlastTheWatchdog(page) { async function outlastTheWatchdog(page) {
// A negative assertion about the watchdog is vacuous where the watchdog was // A negative assertion about the watchdog is vacuous where it was never
// never armed, and `shell.arm` runs only after `await transport.ready`. It // armed. It asks whether it was EVER armed: by here a correct page has
// asks whether it was EVER armed: by here a correct page has disarmed it, // disarmed it, which is the property, so `armed()` would deny the passing
// which is the property, so `armed()` would deny the passing case. // case. Since improvise-vep8 arming precedes `transport.ready`.
expect( expect(
await page.evaluate(() => window.__improviseShell?.timesArmed() ?? 0), await page.evaluate(() => window.__improviseShell?.timesArmed() ?? 0),
"the watchdog was never armed on this path: denying its message proves nothing", "the watchdog was never armed on this path: denying its message proves nothing",
@@ -74,8 +74,8 @@ test.describe("the shell reports what its peer is doing", { tag: "@mocked-peer"
await expect(status(page)).toContainText("bad frame", { timeout: 15_000 }); await expect(status(page)).toContainText("bad frame", { timeout: 15_000 });
}); });
// improvise-hwpi. This peer NEVER OPENS, so `await transport.ready` never // improvise-hwpi. This peer never opens; since improvise-vep8 the watchdog
// resolves and `shell.arm` never runs — the watchdog is not on this path. // IS armed here, and `error`/`close` disarm it with the precise reason.
test("a peer that cannot be reached is named, and not called a disconnect", async ({ test("a peer that cannot be reached is named, and not called a disconnect", async ({
page, page,
}) => { }) => {
@@ -119,14 +119,30 @@ test.describe("the shell reports what its peer is doing", { tag: "@mocked-peer"
}); });
// The one assertion that needs the real timer: that the watchdog is ARMED // The one assertion that needs the real timer: that the watchdog is ARMED
// after the handshake goes out. What it then says, and that a loud failure // and speaks. Since improvise-vep8 it is armed BEFORE `transport.ready`, so
// disarms it, are unit tests. The mock accepts and never answers. // its message covers a peer that never opened as well as one that went
// quiet — which is why it no longer claims the connection was accepted.
test("a peer that accepts and then says nothing trips the watchdog", async ({ test("a peer that accepts and then says nothing trips the watchdog", async ({
page, page,
}) => { }) => {
await page.routeWebSocket("ws://127.0.0.1:8788/", () => {}); await page.routeWebSocket("ws://127.0.0.1:8788/", () => {});
await page.goto("/app/index.html?ws=127.0.0.1:8788"); await page.goto("/app/index.html?ws=127.0.0.1:8788");
await expect(status(page)).toContainText("sent no document", { await expect(status(page)).toContainText("never answered", {
timeout: HANDSHAKE_TIMEOUT_MS + 12_000,
});
});
// improvise-vep8: a peer that never OPENS never resolves `transport.ready`,
// so arming after it left this path with no watchdog at all. The socket is
// mocked to hang, so neither `error` nor `close` ever fires to cover for it.
test("a connection that hangs forever is still caught by the watchdog", async ({
page,
}) => {
// Accepts the TCP/WS handshake at the Playwright level but never lets the
// page see `open`: the page sits in `await transport.ready`.
await page.routeWebSocket("ws://127.0.0.1:8791/", () => new Promise(() => {}));
await page.goto("/app/index.html?ws=127.0.0.1:8791");
await expect(status(page)).toContainText("never answered", {
timeout: HANDSHAKE_TIMEOUT_MS + 12_000, timeout: HANDSHAKE_TIMEOUT_MS + 12_000,
}); });
}); });
@@ -146,6 +162,40 @@ test.describe("the standalone deployment", () => {
await expect(page.locator("#grid td.editable").first()).toBeVisible(); await expect(page.locator("#grid td.editable").first()).toBeVisible();
}); });
// improvise-9cma: a `?doc=` this backend cannot hold used to reach the
// generic watchdog 8s later. The REASON now crosses the port (improvise-1wft).
test("an invalid ?doc= is named on screen, and not blamed on the browser", async ({
page,
}) => {
await page.clock.install();
await page.addInitScript(logStatus);
await page.goto("/app/index.html?doc=a/b/c.improv");
await expect(status(page)).toContainText("could not start", { timeout: 20_000 });
await expect(status(page)).toContainText("does not name a document");
// The property is that the precise reason ARRIVES and STANDS: the watchdog
// must not replace it eight seconds later (gotcha 48).
await outlastTheWatchdog(page);
await expect(status(page)).toContainText("does not name a document");
const log = await page.evaluate(() => window.__statusLog);
expect(
log.some((t) => t.includes("did not answer")),
`#status said: ${json(log)}`,
).toBe(false);
});
// improvise-iobc: an empty document is indistinguishable from an intended
// one unless the page says the seed is what failed.
test("a seed that 404s says so rather than showing a silent empty grid", async ({
page,
}) => {
await page.goto("/app/index.html?doc=no-seed.improv&seed=nope.improv");
// It still OPENS — an unfetchable seed is not a fatal boot failure.
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await expect(status(page)).toContainText("EMPTY");
await expect(status(page)).toContainText("nope.improv");
});
// What lands on screen is the HOST's value, not the string typed — the // What lands on screen is the HOST's value, not the string typed — the
// round trip through a real worker, which no unit test can stand in for. // round trip through a real worker, which no unit test can stand in for.
test("committing an edit sends one frame and shows the host's value", async ({ test("committing an edit sends one frame and shows the host's value", async ({
+2 -1
View File
@@ -3,6 +3,7 @@
import init, { import init, {
run_checks, run_checks,
run_probes,
run_canary, run_canary,
before_reload, before_reload,
after_reload, after_reload,
@@ -10,7 +11,7 @@ import init, {
const ready = init(); const ready = init();
const ENTRY = { run_checks, run_canary, before_reload, after_reload }; const ENTRY = { run_checks, run_probes, run_canary, before_reload, after_reload };
self.onmessage = async (event) => { self.onmessage = async (event) => {
const { id, cmd } = event.data; const { id, cmd } = event.data;
+52 -10
View File
@@ -6,12 +6,15 @@ import {
COL_PX, COL_PX,
HANDSHAKE_TIMEOUT_MS, HANDSHAKE_TIMEOUT_MS,
ROW_PX, ROW_PX,
classifyWorkerMessage,
createShell, createShell,
documentTitle, documentTitle,
extentOf, extentOf,
frameCostOf, frameCostOf,
gridPlan, gridPlan,
indicator, indicator,
isFatal,
noticeText,
socketUrl, socketUrl,
} from "./shell.js"; } from "./shell.js";
@@ -64,7 +67,18 @@ function sendViewport() {
const { rows, cols } = extentOf(box, frameCost, metrics); const { rows, cols } = extentOf(box, frameCost, metrics);
// `set_viewport` RECORDS the window as well as framing it; the frame-only // `set_viewport` RECORDS the window as well as framing it; the frame-only
// twin beside it is deleted, so there is nothing to call by mistake (seik). // twin beside it is deleted, so there is nothing to call by mistake (seik).
transport.send(client.set_viewport(rows, cols)); // QUIET: an extent announcement is not a user edit, and a `ResizeObserver`
// firing after a drop must not overwrite the precise close message.
transport.send(client.set_viewport(rows, cols), { loud: false });
}
/** What the worker said that was not a frame: a boot or save failure carrying
* its REASON, which is the channel improvise-1wft exists to add. */
function onNotice(notice) {
// A notice is a LOUD failure, so it disarms the watchdog rather than being
// replaced by it eight seconds later (gotcha 48).
shell.fail(noticeText(notice));
if (isFatal(notice)) console.error("improvise: boot failed", notice);
} }
/** One frame, one fold, in arrival order. Never batch or reorder these. */ /** One frame, one fold, in arrival order. Never batch or reorder these. */
@@ -90,7 +104,14 @@ function workerTransport() {
// `type: "module"` is not optional: worker.js is an ES module (it `import`s // `type: "module"` is not optional: worker.js is an ES module (it `import`s
// the worker bundle), and a classic worker would fail to parse it. // the worker bundle), and a classic worker would fail to parse it.
const worker = new Worker(`./worker.js?${query}`, { type: "module" }); const worker = new Worker(`./worker.js?${query}`, { type: "module" });
worker.addEventListener("message", (event) => onFrame(event.data)); worker.addEventListener("message", (event) => {
// A protocol frame is a string; a shell notice is an object. The rule is
// `shell.js`'s and is tested there.
const message = classifyWorkerMessage(event.data);
if (message.kind === "frame") onFrame(message.text);
else if (message.kind === "notice") onNotice(message.notice);
else console.error("improvise: worker sent an unreadable message", event.data);
});
worker.addEventListener("messageerror", () => worker.addEventListener("messageerror", () =>
shell.fail("the worker sent a frame this page could not read"), shell.fail("the worker sent a frame this page could not read"),
); );
@@ -129,8 +150,21 @@ function socketTransport() {
); );
shell.setShell(`connecting to ${WS_URL}…`); shell.setShell(`connecting to ${WS_URL}…`);
return { return {
send: (text) => { // CLOSING can THROW and CONNECTING/CLOSED drop silently, which loses an
if (socket.readyState === WebSocket.OPEN) socket.send(text); // edit with nothing on screen to say so (improvise-ociy).
send: (text, { loud = true } = {}) => {
if (socket.readyState !== WebSocket.OPEN) {
if (loud) {
shell.fail(`not sent — ${opened ? "the connection dropped" : "not connected yet"}`);
}
return;
}
try {
socket.send(text);
} catch (err) {
if (loud) shell.fail("not sent — the connection is closing");
console.error("improvise: send failed", err);
}
}, },
ready, ready,
}; };
@@ -142,14 +176,20 @@ const transport = WS_URL ? socketTransport() : workerTransport();
let painted = -1; let painted = -1;
let editing = null; // {row, col} while an input is open, so a repaint cannot steal it let editing = null; // {row, col} while an input is open, so a repaint cannot steal it
// A render that threw is not retried: `painted` is unchanged on that path, so
// every later event re-ran the same failure forever (improvise-7gf8).
let renderBroken = false;
function repaint() { function repaint() {
if (editing) return; if (editing || renderBroken) return;
let model; let model;
try { try {
model = JSON.parse(client.render_model()); model = JSON.parse(client.render_model());
} catch (err) { } catch (err) {
shell.setShell("cannot render the current state (see console)"); // Tripped, not counted: a render model that cannot be built is a permanent
// state, and a reload is the only recovery this page has.
renderBroken = true;
shell.fail("cannot render the current state — reload the page (see console)");
console.error("improvise: render failed", err); console.error("improvise: render failed", err);
return; return;
} }
@@ -486,11 +526,13 @@ window.addEventListener("keydown", (e) => {
// Start. // Start.
await transport.ready; // ARMED FIRST. `transport.ready` never resolves for a socket that never
sendViewport(); // the handshake: the snapshot is built at this extent // opens, so arming after it left a hang with no watchdog at all (improvise-vep8).
shell.arm( shell.arm(
WS_URL WS_URL
? `${WS_URL} accepted the connection but sent no document` ? `${WS_URL} never answered — is improvise-ws-server running, and did it send a document?`
: "the in-page worker did not answer — this browser may not allow it (see console)", : "the in-page worker did not answer (see console)",
); );
await transport.ready;
sendViewport(); // the handshake: the snapshot is built at this extent
new ResizeObserver(sendViewport).observe(el.grid); new ResizeObserver(sendViewport).observe(el.grid);
+37
View File
@@ -112,6 +112,43 @@ export function gridPlan(model) {
return { cornerSpan, groups, levels: model.col_levels, body }; return { cornerSpan, groups, levels: model.col_levels, body };
} }
/** What the worker just posted. A protocol frame is a STRING and a shell
* notice is an OBJECT; nothing else crosses that port (clients.md). */
export function classifyWorkerMessage(data) {
if (typeof data === "string") return { kind: "frame", text: data };
if (data && typeof data === "object" && typeof data.kind === "string") {
return { kind: "notice", notice: data };
}
return { kind: "unknown", data };
}
/** What a boot or save notice READS AS. The kinds are decided in Rust
* (`improvise_worker_server::boot`); only the wording is the page's. */
export function noticeText(notice) {
const where = notice.document ? ` — ${notice.document}` : "";
const why = notice.detail ? ` (${notice.detail})` : "";
switch (notice.kind) {
case "ReadOnly":
return `read-only${where}: another tab or window has it open. Close it and reload to edit.`;
case "BootFailed":
return `the in-page worker could not start${where}${why}`;
case "SeedMissing":
return `started an EMPTY document${where}: its starting copy could not be fetched${why}`;
case "SaveFailed":
return `NOT SAVED${where}${why}`;
// A kind this page does not know is still a failure, and saying so beats
// saying nothing — the reason it carries is the point (improvise-1wft).
default:
return `the in-page worker reported ${notice.kind}${where}${why}`;
}
}
/** Whether a notice means the session is unusable, as opposed to degraded.
* Only `BootFailed` is fatal: read-only, empty and unsaved all still paint. */
export function isFatal(notice) {
return notice.kind === "BootFailed";
}
/** The page's two status facts and the handshake watchdog, which share a home /** The page's two status facts and the handshake watchdog, which share a home
* because the one rule between them is that a *failure disarms the timer*. */ * because the one rule between them is that a *failure disarms the timer*. */
export function createShell({ onStatus, timeoutMs, setTimer = setTimeout, clearTimer = clearTimeout }) { export function createShell({ onStatus, timeoutMs, setTimer = setTimeout, clearTimer = clearTimeout }) {
+3 -2
View File
@@ -123,6 +123,7 @@ test("every worker call web/worker.js makes is declared, with that arity", () =>
`server.${call.name} takes ${arity[0]}..${arity[1]}; worker.js passes ${call.args}`, `server.${call.name} takes ${arity[0]}..${arity[1]}; worker.js passes ${call.args}`,
); );
} }
// `start` is a free function rather than a method, and takes the seed. // `start` is a free function rather than a method, and takes the seed AND
assert.deepEqual(declared.get("start"), [1, 2]); // why it is missing, so an empty document can say so (improvise-iobc).
assert.deepEqual(declared.get("start"), [1, 3]);
}); });
+76
View File
@@ -5,12 +5,15 @@ import { test } from "node:test";
import assert from "node:assert/strict"; import assert from "node:assert/strict";
import { import {
classifyWorkerMessage,
composeStatus, composeStatus,
createShell, createShell,
documentTitle, documentTitle,
extentOf, extentOf,
frameCostOf, frameCostOf,
indicator, indicator,
isFatal,
noticeText,
socketUrl, socketUrl,
} from "../shell.js"; } from "../shell.js";
@@ -175,3 +178,76 @@ test("the title bar spells the document the way the TUI's own title does", () =>
// arrives as `null`, and an empty pair of parens would be a lie about storage. // arrives as `null`, and an empty pair of parens would be a lie about storage.
assert.equal(documentTitle("Untitled", true, null), "Untitled [+]"); assert.equal(documentTitle("Untitled", true, null), "Untitled [+]");
}); });
test("a protocol frame is a string and a shell notice is an object", () => {
assert.deepEqual(classifyWorkerMessage('{"Commands":[]}'), {
kind: "frame",
text: '{"Commands":[]}',
});
const notice = { kind: "ReadOnly", document: "a.improv", detail: "held" };
assert.deepEqual(classifyWorkerMessage(notice), { kind: "notice", notice });
// The discriminating case: a notice serialized as a STRING would be fed to
// the fold, which is exactly the confusion the two shapes exist to prevent.
assert.equal(classifyWorkerMessage(JSON.stringify(notice)).kind, "frame");
});
test("a message that is neither is named rather than silently dropped", () => {
for (const junk of [null, undefined, 42, {}, { detail: "no kind" }]) {
assert.equal(
classifyWorkerMessage(junk).kind,
"unknown",
`classified ${JSON.stringify(junk)}`,
);
}
});
test("every notice reads as a reason, and names its document", () => {
const of = (kind) => noticeText({ kind, document: "books.improv", detail: "404" });
// improvise-i0v9: the page must say what is wrong, not blame the browser.
assert.match(of("ReadOnly"), /read-only/);
assert.match(of("ReadOnly"), /another tab/);
// improvise-iobc: an empty document must not pass for an intentional one.
assert.match(of("SeedMissing"), /EMPTY/);
assert.match(of("SeedMissing"), /404/);
// improvise-zre1: "not saved" must not read like "saving".
assert.match(of("SaveFailed"), /NOT SAVED/);
assert.match(of("BootFailed"), /could not start/);
for (const kind of ["ReadOnly", "SeedMissing", "SaveFailed", "BootFailed"]) {
assert.match(of(kind), /books\.improv/, `${kind} did not name the document`);
}
});
test("a notice kind this page has never heard of still carries its reason", () => {
const text = noticeText({ kind: "SomethingNew", document: "a.improv", detail: "because" });
assert.match(text, /SomethingNew/);
assert.match(text, /because/);
});
test("only a failed boot is fatal — read-only, empty and unsaved all still paint", () => {
assert.equal(isFatal({ kind: "BootFailed" }), true);
for (const kind of ["ReadOnly", "SeedMissing", "SaveFailed"]) {
assert.equal(isFatal({ kind }), false, `${kind} was treated as fatal`);
}
});
test("a notice DISARMS the watchdog rather than being replaced by it", () => {
const clock = fakeClock();
const seen = [];
const shell = createShell({
onStatus: (t) => seen.push(t),
timeoutMs: 8000,
setTimer: clock.setTimer,
clearTimer: clock.clearTimer,
});
// The order improvise-vep8 puts them in: armed first, then the reason.
shell.arm("the in-page worker did not answer (see console)");
assert.equal(shell.timesArmed(), 1, "the denial below is vacuous if it was never armed");
shell.fail(noticeText({ kind: "ReadOnly", document: "a.improv", detail: "" }));
clock.fire();
assert.match(seen.at(-1), /read-only/);
assert.equal(
seen.some((t) => t.includes("did not answer")),
false,
`the watchdog spoke over a precise reason: ${JSON.stringify(seen)}`,
);
});
+31 -9
View File
@@ -10,15 +10,15 @@ const DOC = params.get("doc") || "demo.improv";
// and WRITTEN through the VFS, never injected past storage. // and WRITTEN through the VFS, never injected past storage.
const SEED = params.get("seed") || "demo.improv"; const SEED = params.get("seed") || "demo.improv";
// Fetch the seed if there is one to fetch. A missing seed is not an error: // Fetch the seed. A failure is NOT decided here: the bytes and the reason both
// with none, the worker creates an empty model under DOC instead. // go to Rust, which knows whether the document needed a first copy at all.
async function seedBytes() { async function seedBytes() {
try { try {
const res = await fetch(SEED); const res = await fetch(SEED);
if (!res.ok) return undefined; if (!res.ok) return { error: `${SEED} answered ${res.status} ${res.statusText}` };
return new Uint8Array(await res.arrayBuffer()); return { bytes: new Uint8Array(await res.arrayBuffer()) };
} catch (_) { } catch (err) {
return undefined; return { error: `${SEED} could not be fetched: ${err?.message ?? err}` };
} }
} }
@@ -28,20 +28,42 @@ function post(replies) {
for (const reply of replies) self.postMessage(reply); for (const reply of replies) self.postMessage(reply);
} }
/** A shell notice, posted as an OBJECT so the page can tell it from a protocol
* frame without parsing one. See `shell.js::classifyWorkerMessage`. */
function notify(notice) {
self.postMessage(notice);
}
async function handle(data) { async function handle(data) {
const replies = server.on_message(data); const replies = server.on_message(data);
post(replies); post(replies);
await server.flush(); await server.flush();
// A persist nobody asked for that did not happen (improvise-zre1). Drained
// per message, so one failure is reported once.
const raised = server.take_notice();
if (raised) notify(JSON.parse(raised));
} }
// Startup, and the head of the chain: frames arrive before this settles, and // Startup, and the head of the chain: frames arrive before this settles, and
// chaining onto it holds them in arrival order with no queue to drain. // chaining onto it holds them in arrival order with no queue to drain.
const started = (async () => { const started = (async () => {
await init(); await init();
server = await start(DOC, await seedBytes()); const seed = await seedBytes();
server = await start(DOC, seed.bytes, seed.error);
// Read once, straight after start: read-only, or an empty document whose
// seed never arrived. Both are open sessions, so this is not a failure.
const boot = server.boot_notice();
if (boot) notify(JSON.parse(boot));
})(); })();
started.catch((err) => { started.catch((err) => {
console.error("improvise worker: failed to start", err); console.error("improvise worker: failed to start", err);
// A worker that rejects in `start()` fires NO error on the main thread, so
// without this the page has only a watchdog and no reason (improvise-1wft).
notify({
kind: "BootFailed",
document: DOC,
detail: String(err?.message ?? err),
});
}); });
// One frame at a time, in arrival order, and the listener is deliberately NOT // One frame at a time, in arrival order, and the listener is deliberately NOT
@@ -49,8 +71,8 @@ started.catch((err) => {
const onFrame = serialize( const onFrame = serialize(
started, started,
async (data) => { async (data) => {
// `start()` failed. There is nothing to answer with, and the main // `start()` failed, and the page has already been told why by the
// thread's handshake watchdog is what tells the user. // `BootFailed` notice above. There is nothing to answer with.
if (!server) return; if (!server) return;
await handle(data); await handle(data);
}, },