From 54f277c69acc0dfd8d91ff5a60b5108a633a7420 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 07:00:34 -0500 Subject: [PATCH 1/2] feat(weft-server): end the process when a control-plane write panics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROB-2 in wave 2, per release plan C12 and C22: an engine-origin panic while a write is in flight ends the process instead of being answered. `weftdb::exec::control_plane_write` was already the write-scope flag the plan asks for, and it already wraps the whole control-plane write path — every `IndexTxn` operation runs inside it, which weftdb's own `every_op_runs_inside_the_control_plane_write_scope` asserts — so the missing half was the hook, and C12 puts the hook in the binary: `weftdb` holds the engine types but never exits a process. `panic_hook::install` chains the hook it replaces, and when `in_control_plane_write()` says the panicking task was inside the region it names the panic and calls `std::process::abort()`. Startup calls it immediately after `init_tracing`, because `set_hook` hands back the hook it replaced: installing ours first would leave the tracing subscriber's hook calling ours and print every panic twice. Abort rather than the poison path's exit 70: that status is chosen by a known condition with a known operator action, and FRE-6a's point is that this failure must not arrive as an HTTP response, so the exit should carry a signal a supervisor sees. The hook also deliberately does not write the poison marker. Most of the region reads before it writes — `txn.plans()` prepares inside the scope, `SegmentIndex::open` runs inside it — so a panic there does not imply a torn commit, and the marker persists in the store directory until an operator deletes a file, which would turn a benign panic into an outage. The abort buys the restart, and the restart already runs the open-time verification that can judge the store. Nothing is caught: a panic outside the region is reported and left unwinding, so wave 6's ROB-3 catch layer stays the only catch layer and read-path panics keep answering 500. The legacy metadata lane (`weftdb/src/types/aspect.rs`) is untouched; the plan gives that to ROB-5. `tests/control_plane_panic.rs` checks both halves in a child process, the way `poison_exit.rs` does: inside the region, through `control_plane_write` with a yield between entry and panic, the child dies by SIGABRT with the panic and the reason on stderr; outside it, in a spawned task, the child exits 0 — the chained hook still printed the panic, the abort line is absent, and the process kept running. --- CHANGELOG.md | 21 +++++- weft-server/src/lib.rs | 1 + weft-server/src/panic_hook.rs | 81 +++++++++++++++++++++ weft-server/src/settings.rs | 5 +- weft-server/src/startup/hooks.rs | 19 ++++- weft-server/src/startup/mod.rs | 5 +- weft-server/tests/control_plane_panic.rs | 92 ++++++++++++++++++++++++ weftdb/src/types/exec.rs | 8 ++- 8 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 weft-server/src/panic_hook.rs create mode 100644 weft-server/tests/control_plane_panic.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d681b9..f719ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,20 @@ only the control plane, not the segment frames. it (unset or `poison` keeps the default); the library itself never exits the process (see the options entry under Changed). `SegmentStore::wait_until_poisoned()` and `subscribe_poison()` let an embedder do the same. +- **A panic during a control-plane write ends the process** (robustness track ROB-2, release + plan C12, C22). `weftdb::exec::control_plane_write` already marks the region that the whole + `segment_index.db` write path runs in; the server now acts on it. `weft_server::panic_hook::install()` + is installed at startup immediately after the tracing subscriber, chained to the hook it + replaces so the panic is still reported with its message and location. A panic whose + panicking task is inside that region is written to stderr and the process + `std::process::abort()`s: the exit carries a signal a supervisor sees (SIGABRT) instead of + an HTTP status, and the restart that follows reopens the store under the engine's own crash + recovery, which is the component that can tell whether a commit was torn. A panic anywhere + else is reported and left unwinding, because wave 6's ROB-3 catch layer is the only layer + that catches panics. The hook does not write the poison marker — most of the region reads + before it writes, so a marker there would turn a benign panic into an outage lasting until + an operator deletes a file. `weft-server/tests/control_plane_panic.rs` proves both halves + by re-executing the test binary as a child, the way `poison_exit.rs` does. - **Store format marker and migration registry** (freeze design §4.3, FRE-12a). A segment store root now carries a `STORE_FORMAT` file (plain JSON: `layout_version`, `min_read_layout`, `min_write_layout`, `last_written_layout`, `migrating_to`, @@ -69,9 +83,10 @@ only the control plane, not the segment frames. - `weftdb::exec::control_plane_write` and `weftdb::durable::poison_global()` (robustness track ROB-2): every segment-index transaction runs inside the task-local control-plane write scope, so a panic hook can tell a panic in the middle - of a control-plane write from one in a read, and the process-wide poison it would set - refuses every write of every store, as a store's own poison does, while reads go on. - It refuses every store open too (`weftdb::Poisoned`), since an open writes. + of a control-plane write from one in a read. `poison_global()` is a process-wide + poison that, once set, refuses every write of every store, as a store's own poison + does, while reads go on. It refuses every store open too (`weftdb::Poisoned`), since + an open writes. - `LOCK.holder` records the holder's host name and boot id beside its pid, and a second opener's error names the host (`… in use by pid 1 on host db-2 (session …)`). The short wait for a lock that a child being spawned still shares now applies only to diff --git a/weft-server/src/lib.rs b/weft-server/src/lib.rs index 421ec5b..1e6d655 100644 --- a/weft-server/src/lib.rs +++ b/weft-server/src/lib.rs @@ -42,6 +42,7 @@ pub mod ingest; pub mod interpolate; pub mod maintain; pub mod metrics; +pub mod panic_hook; pub mod reconcile_daemon; pub mod settings; pub mod state; diff --git a/weft-server/src/panic_hook.rs b/weft-server/src/panic_hook.rs new file mode 100644 index 0000000..3cb63ee --- /dev/null +++ b/weft-server/src/panic_hook.rs @@ -0,0 +1,81 @@ +//! The process-wide panic hook (robustness track **ROB-2**, release plan C12, C22). +//! +//! A panic in the middle of a control-plane write is not like a panic in a read: Turso's +//! pager may be part way through a commit, its locks do not poison, and the task that +//! panics is holding a `&mut` borrow of state that the rest of the process still believes +//! is consistent. Unwinding lets the request finish as a 500 and lets the *next* request +//! answer as if nothing happened — a write that was half-issued reporting success, which is +//! the condition C22 exists to prevent. So: +//! +//! - a panic while the task is inside [`weftdb::exec::control_plane_write`] is reported on +//! stderr and then the process **aborts**, so the exit carries a signal a supervisor sees +//! (SIGABRT) instead of an HTTP response (FRE-6a); the restart that follows opens the +//! store under the engine's own crash recovery, which is the component that actually +//! knows whether a commit was torn; +//! - any other panic is reported and left unwinding — ROB-3's catch layer (wave 6) is the +//! only layer that catches panics, and it needs unwinding to stay reachable. +//! +//! Two things this deliberately is not: +//! +//! - It does not write the poison marker (`weftdb::durable::poison_global`). A panic inside +//! the region can precede any commit at all — most of the region reads before it writes — +//! so the marker would turn a benign panic into an outage lasting until an operator +//! deletes a file. Failing the process loudly is the fail-stop; judging the store is left +//! to the open-time verification that already runs on every start. +//! - It does not catch anything, and the library still never exits a process: this hook +//! lives in `weft-server`, the same split as the poison path, where `weftdb` reports and +//! the host decides how the process ends. +//! +//! Install it from the binary at startup, **after** the tracing subscriber: the hook chains +//! the hook it replaces, so ordering decides who prints the panic. + +use weftdb::exec::in_control_plane_write; + +/// Whether [`install`] has run in this process. Startup calls `install` unconditionally, and +/// a test that installs the hook first would otherwise replace it and print the panic twice +/// through two chained hooks. +static INSTALLED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + +/// Install the ROB-2 panic hook, chaining whatever hook is already installed so the panic +/// itself is still reported with its message and location. +/// +/// Idempotent: a second call does nothing, which is what lets a test install the hook +/// directly and the binary's startup path skip it. +pub fn install() { + if INSTALLED.swap(true, std::sync::atomic::Ordering::SeqCst) { + return; + } + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + // First report, in the form the operator expects: message, file, line, and the + // span the panic happened in. + previous(info); + if in_control_plane_write() { + // Written to stderr rather than traced: the panicking task may be holding the + // pager's state, and this line has to reach whoever reads the crash log even + // when the logging pipeline is itself what failed. One line, so a log shipper + // that splits on newlines cannot separate the reason from the panic. + eprintln!("weft-server: the control plane was being written when this panic happened — aborting the process: {}", shown(info)); + std::process::abort(); + } + })); +} + +/// The panic's payload and location, for the abort line. +/// +/// Payload types are arbitrary and their `Display` can panic, which inside a panic hook +/// would abort the hook itself, so an unusable payload is reported as its type instead. +fn shown(info: &std::panic::PanicHookInfo<'_>) -> String { + let payload = info.payload(); + let payload = if let Some(text) = payload.downcast_ref::<&str>() { + (*text).to_owned() + } else if let Some(text) = payload.downcast_ref::() { + text.clone() + } else { + "a panic whose payload could not be printed".to_owned() + }; + match info.location() { + Some(location) => format!("{payload} (at {}:{})", location.file(), location.line()), + None => payload, + } +} diff --git a/weft-server/src/settings.rs b/weft-server/src/settings.rs index b3f9001..ac68d31 100644 --- a/weft-server/src/settings.rs +++ b/weft-server/src/settings.rs @@ -131,6 +131,7 @@ pub const VARS: &[VarSpec] = &[ VarSpec { name: "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process the process-wide-poison test spawns: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" }, VarSpec { name: "WEFT_TEST_OPEN_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process the open-recovery test spawns: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" }, VarSpec { name: "WEFT_TEST_ORPHAN_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process that seals a frame and crashes before its commit: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" }, + VarSpec { name: "WEFT_TEST_PANIC_CHILD_MODE", tier: Tier::Test, summary: "Names which half of the control-plane panic test the child process is exercising, so its parent can tell a deliberate abort from a real crash. No shipped binary reads it.", accepts: "`inside` panics while the control-plane write scope is held; `outside` panics in an ordinary task", default: "unset: the child returns and the test binary exits 0", read_by: "weft-server/tests/control_plane_panic.rs (test child)" }, VarSpec { name: "WEFT_TEST_POISON_EXIT_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child `weft-server` the poison-exit test re-invokes: the store root it opens, so the parent can point the child at the poisoned store.", accepts: "a directory path", default: "unset", read_by: "weft-server/tests/poison_exit.rs (test child)" }, VarSpec { name: "WEFT_TEST_RACE_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process that reconciles while a squash races it: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" }, VarSpec { name: "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process whose reconcile is merged by a racing squash: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store/swap_tests.rs (test child)" }, @@ -363,7 +364,7 @@ mod tests { assert!(!value.trim().is_empty(), "{} describes its {field} as empty", spec.name); } } - assert_eq!(VARS.len(), 51, "the registry's size is asserted so an entry added or dropped is a deliberate act"); + assert_eq!(VARS.len(), 52, "the registry's size is asserted so an entry added or dropped is a deliberate act"); } /// The registry's whole point: it cannot drift from the code. A name read but not @@ -400,7 +401,7 @@ mod tests { let experimental: Vec<&str> = VARS.iter().filter(|spec| spec.tier == Tier::Experimental).map(|spec| spec.name).collect(); assert_eq!(experimental, ["WEFT_GPU_CALIBRATE", "WEFT_SEGMENT_TRANSPOSED_MAX_OVERHEAD"], "both are knobs whose value is unproven (GPU calibration's thresholds, and a codec the measurements retired)"); let harness: BTreeSet<&str> = VARS.iter().filter(|spec| spec.tier == Tier::Test).map(|spec| spec.name).collect(); - let allowlist: BTreeSet<&str> = ["SKIP_SLOW_TESTS", "TEST_DATA_DIR", "WEFT_FAULT", "WEFT_FAULT_TEST_CHILD", "WEFT_REQUIRE_GPU", "WEFT_TEST_AMBIGUOUS_CHILD_ROOT", "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", "WEFT_TEST_OPEN_CHILD_ROOT", "WEFT_TEST_ORPHAN_CHILD_ROOT", "WEFT_TEST_POISON_EXIT_CHILD_ROOT", "WEFT_TEST_RACE_CHILD_ROOT", "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", "WEFT_TEST_S8_SPLIT_RACE_ROOT", "WEFT_TEST_S9_DROPPED_SWAP_DIR", "WEFT_TEST_S9_SQUASH_RACE_DIR", "WEFT_TEST_S9_SUFFIX_RACE_DIR", "WEFT_TEST_SWAP_MATRIX", "WEFT_TEST_SWAP_MATRIX_DIR", "WEFT_TEST_TXN_CHILD_INDEX", "WEFT_BENCH_BACKUP_DIR", "WEFT_BTC_CSV", "WEFT_DB_NEW_CRASH_CHILD", "WEFT_LEGACY_CONSUMER_CRASH_CHILD", "WEFT_LEGACY_INGEST_CRASH_CHILD", "WEFT_VERBOSE"].into_iter().collect(); + let allowlist: BTreeSet<&str> = ["SKIP_SLOW_TESTS", "TEST_DATA_DIR", "WEFT_FAULT", "WEFT_FAULT_TEST_CHILD", "WEFT_REQUIRE_GPU", "WEFT_TEST_AMBIGUOUS_CHILD_ROOT", "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", "WEFT_TEST_OPEN_CHILD_ROOT", "WEFT_TEST_ORPHAN_CHILD_ROOT", "WEFT_TEST_PANIC_CHILD_MODE", "WEFT_TEST_POISON_EXIT_CHILD_ROOT", "WEFT_TEST_RACE_CHILD_ROOT", "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", "WEFT_TEST_S8_SPLIT_RACE_ROOT", "WEFT_TEST_S9_DROPPED_SWAP_DIR", "WEFT_TEST_S9_SQUASH_RACE_DIR", "WEFT_TEST_S9_SUFFIX_RACE_DIR", "WEFT_TEST_SWAP_MATRIX", "WEFT_TEST_SWAP_MATRIX_DIR", "WEFT_TEST_TXN_CHILD_INDEX", "WEFT_BENCH_BACKUP_DIR", "WEFT_BTC_CSV", "WEFT_DB_NEW_CRASH_CHILD", "WEFT_LEGACY_CONSUMER_CRASH_CHILD", "WEFT_LEGACY_INGEST_CRASH_CHILD", "WEFT_VERBOSE"].into_iter().collect(); assert_eq!(harness, allowlist, "the Test tier is the plan's allowlist: a channel a test harness sets for the child it spawns, and nothing else may sit in it"); assert_eq!(VARS.iter().filter(|spec| spec.tier == Tier::Tuning).count(), VARS.len() - stable.len() - experimental.len() - allowlist.len(), "every remaining name is Tuning: a knob whose value is deployment-specific and whose number is still moving"); } diff --git a/weft-server/src/startup/hooks.rs b/weft-server/src/startup/hooks.rs index d8be657..fadd14a 100644 --- a/weft-server/src/startup/hooks.rs +++ b/weft-server/src/startup/hooks.rs @@ -1,10 +1,14 @@ -//! Startup phase 1 — the process-wide hooks: the tracing subscriber, and the OTLP -//! exporter when the operator configured a collector. +//! Startup phase 1 — the process-wide hooks: the tracing subscriber, the OTLP exporter +//! when the operator configured a collector, and the panic hook. //! //! Installed before anything else runs, so every later startup phase and every //! request is observable. Installing a subscriber cannot fail the server: a //! `try_init` is a no-op when one is already installed, and a collector that //! cannot be reached falls back to the `fmt`-only path. +//! +//! The panic hook is installed **after** the subscriber because it reports through it: +//! [`std::panic::set_hook`] replaces the installed hook and hands the replaced one back, so +//! the order decides who prints the panic. See [`init_panic_hook`]. use weft_server::SERVICE; @@ -49,6 +53,17 @@ pub(crate) fn init_tracing() -> Option anyhow::Result<()> { // early would tear down the batch span processor and stop export. It is shut down on // a clean exit to flush any buffered spans. let otel_provider = hooks::init_tracing(); + // Immediately after tracing, so the panic hook reports through the subscriber it chains. + hooks::init_panic_hook(); let addr = config::bind_addr()?; let interpolate = config::interpolate_config()?; diff --git a/weft-server/tests/control_plane_panic.rs b/weft-server/tests/control_plane_panic.rs new file mode 100644 index 0000000..04322cd --- /dev/null +++ b/weft-server/tests/control_plane_panic.rs @@ -0,0 +1,92 @@ +//! Robustness track **ROB-2** (release plan C12, C22): a panic while the process is +//! writing the control plane ends the process instead of being answered. +//! +//! [`weftdb::exec::control_plane_write`] marks the region — weftdb's own test +//! `every_op_runs_inside_the_control_plane_write_scope` proves every op of every +//! segment-index transaction runs inside it — and [`weft_server::panic_hook`] is the hook +//! that reads the mark and aborts. This test checks both halves from outside the library: +//! a panic **inside** the region kills the process with a signal a supervisor can see and +//! says why on stderr, and a panic **outside** it still only ends its own task, because +//! ROB-3's catch layer (wave 6) is the only layer that catches panics and unwinding must +//! stay reachable for it. +//! +//! The abort ends whatever process runs it, so each case runs in a child: this test binary +//! re-executed with one test selected, exactly as `poison_exit.rs` does. +//! +//! Run with `cargo test -p weft-server --test control_plane_panic`. + +use weft_server::panic_hook::install; + +/// Set only in the child: which panic to raise. +const CHILD_MODE: &str = "WEFT_TEST_PANIC_CHILD_MODE"; +/// The child panics inside a control-plane write. +const INSIDE: &str = "inside"; +/// The child panics in a task that is writing nothing. +const OUTSIDE: &str = "outside"; +/// What the child panics with, inside the region. +const INJECTED_INSIDE: &str = "injected panic in the middle of a control-plane write"; +/// What the child panics with, outside it. +const INJECTED_OUTSIDE: &str = "injected panic in a task that is not writing the control plane"; +/// What the child prints when a panic outside the region left it running. +const SURVIVED: &str = "panic child: still running after the panic"; + +/// The child's body: install the server's panic hook, then panic in one of the two places. +/// In a normal test run the variable is unset and this does nothing. +#[tokio::test] +async fn control_plane_panic_child() { + let mode = std::env::var(CHILD_MODE).unwrap_or_default(); + if mode != INSIDE && mode != OUTSIDE { + return; + } + install(); + if mode == INSIDE { + // The same future the store's writes run in: yielded through, so the mark is one + // the hook reads from a later poll on a possibly different worker thread. + weftdb::exec::control_plane_write(async { + tokio::task::yield_now().await; + panic!("{INJECTED_INSIDE}"); + }) + .await; + return; + } + let task = tokio::spawn(async { panic!("{INJECTED_OUTSIDE}") }); + let error = task.await.expect_err("the task aborts").into_panic(); + let shown = error.downcast_ref::().cloned().unwrap_or_else(|| format!("{error:?}")); + assert!(shown.contains(INJECTED_OUTSIDE), "the panic that ended the task: {shown}"); + println!("{SURVIVED}"); +} + +/// Run [`control_plane_panic_child`] in a child process panicking in `mode`. +async fn run_child(mode: &str) -> std::process::Output { + let exe = std::env::current_exe().expect("finds the test binary"); + let mut child = tokio::process::Command::new(exe); + child.args(["control_plane_panic_child", "--exact", "--nocapture", "--test-threads=1"]).env(CHILD_MODE, mode); + child.output().await.expect("runs the child") +} + +#[tokio::test] +async fn a_panic_inside_a_control_plane_write_aborts_the_process() { + let out = run_child(INSIDE).await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!out.status.success(), "the child did not die: {out:?}"); + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt as _; + assert_eq!(out.status.signal(), Some(6), "it died by SIGABRT (6), not by a status a supervisor might mistake for a clean exit: {out:?}"); + } + assert!(stderr.contains(INJECTED_INSIDE), "the panic itself is on stderr: {stderr}"); + assert!(stderr.contains("the control plane was being written when this panic happened") && stderr.contains("aborting the process"), "and so is the reason it died: {stderr}"); + assert!(!stdout.contains(SURVIVED), "the child never got past the panic: {stdout}"); +} + +#[tokio::test] +async fn a_panic_outside_a_control_plane_write_only_ends_its_task() { + let out = run_child(OUTSIDE).await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "the process lives, so ROB-3 can still catch this case: {out:?}"); + assert!(stderr.contains(INJECTED_OUTSIDE), "the panic is still reported by the chained hook: {stderr}"); + assert!(!stderr.contains("the control plane was being written when this panic happened"), "the abort path is not taken: {stderr}"); + assert!(stdout.contains(SURVIVED), "the process kept running: {stdout}"); +} diff --git a/weftdb/src/types/exec.rs b/weftdb/src/types/exec.rs index 1a1d3b4..297a2ef 100644 --- a/weftdb/src/types/exec.rs +++ b/weftdb/src/types/exec.rs @@ -8,8 +8,12 @@ //! `CONTROL_PLANE_WRITE.try_with(|f| f.get())` (or [`in_control_plane_write`]). Every //! segment-index transaction (`IndexTxn`, the store's whole `segment_index.db` write //! path) runs inside it; a hook that finds the flag set write-poisons the process -//! ([`poison_global`](crate::durable::poison_global)) instead of letting it go on writing -//! through that pager. +//! ([`poison_global`](crate::durable::poison_global)) and stops the process from going on +//! writing through that pager — `weft-server`'s hook does exactly that and aborts (ROB-2). +//! The scope is a task-local, not a thread-local, so it follows the task across `await` +//! points and worker threads. It is **not** inherited by `tokio::spawn`, which is why the +//! region wraps whole operations rather than individual statements: work a transaction +//! spawned out of the region would run unmarked, and the tests below check that. use std::{cell::Cell, future::Future}; From df21a2e18ba6fc97e42d6071bb3e1d74980618b2 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 07:14:39 -0500 Subject: [PATCH 2/2] =?UTF-8?q?docs(roadmap):=20tick=20ROB-2=20=E2=80=94?= =?UTF-8?q?=20a=20panic=20in=20the=20middle=20of=20a=20write=20ends=20the?= =?UTF-8?q?=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The box's wording names the mechanism ("a turso-origin panic aborts") but not the boundary it hangs on, so the note records that the scope was already in the library (`weftdb::exec`'s control-plane write flag, with weftdb's own test proving every index-transaction operation runs inside it) and that #92 supplied only the hook C12 puts in the binary — plus the two decisions a reviewer should be able to disagree with: abort rather than the poison path's exit 70, and no poison marker for a panic that may have happened during the reading half of the region. The arrow marks the split the plan keeps: ROB-5 wraps the legacy metadata lane (`weftdb/src/types/aspect.rs`), which this change leaves alone. --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 79498e5..d022587 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1157,7 +1157,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [x] → **D-S9** Write-once maintenance II: overlap merge, squash, compaction (+ P1/P2 output ids) — #72 - [x] **X-ERR** One error envelope and code table in `error.rs` (absorbs SEC-2, FRE-6a, ROB-1, TAG-0's envelope) — *#90: `weft-server/src/error.rs` (685 lines) is the one module that names a refusal. `Code`'s 42 variants are asserted by name **and** status against the plan's §4 table, and the nine synonyms that table drops (`invalid_request`, `unauthorized`, `store_unconfigured`, `maintenance_busy`, `limit_exceeded`, `panic`, `segment_invalid`, `not_found`, `too_large`) are asserted absent, so a later lane cannot quietly reintroduce one. The envelope is `{"error","code","request_id"}` plus optional `details`/`limit`/`committed` on **every** path: `StrictJson`/`StrictQuery` replace axum's extractors, so a wrong `Content-Type` is `unsupported_media_type` and a malformed body or query is `invalid_json`/`bad_request` instead of axum's empty 400, and `Router::fallback` answers `route_not_found` naming the method, the path, where the aspect list is and where `docs/INTEGRATION.md` is. Classification is by **type**: `classify_read`, `classify_ingest` and `classify_typed` downcast `weftdb::InvalidAspectName`, `weftdb::StoreError`, `SealError`, `ReduceError` and `SketchError`, and the four `SegmentStore` seal wrappers (`weftdb/src/types/segment_store.rs:2782/2798/2814/2826`) now wrap with `anyhow::Error::from(error).context(format!("seal failed: {shown}"))` — `anyhow`'s `Display` prints only the outermost context, so the sentence stays byte-identical while `downcast_ref::()` starts working; a reworded wrapper can no longer move a status. Two message rules survive, each commented with the slice that retires it: `no declared schema` (D-S13 types it as `UnknownAspect`, a store-lane edit) and `out-of-order timestamp` (`weft-arrow-store/src/lib.rs:233`'s untyped `bail!`, D-S10/D-S14). `ConvertError` is direction-dependent, so there are two classifiers and `weft-server/tests/error_contract.rs` asserts both halves of the same `Parquet(…)` — a client `400` ingesting, a server `500` reading. Statuses split where §4 names a different condition: `ToleranceExceeded` is `422 value_not_representable` with `details.max_abs_error`/`details.tolerance` (was a `400` carrying a `seal failed` sentence), `Encode(EncodeError::NotFinite)` is `400 not_finite` with `details.row`, a name the store cannot address is `400 invalid_aspect_name` with `details.aspect`/`details.reason`, a store this binary will not open is `503 incompatible_store` (exit 65 in the §4 C7 table), and a downsample `ReduceError` is 422/400 instead of 500. Two findings worth keeping: `weft_physical_type::EncodeError` names only `Overflow` and `NotFinite`, so a wildcard arm beside them was dead code and is gone — a new arm is meant to stop the build where someone picks the §4 code; and `ApiError::message()` returns the cause **by design** (it is the sentence for the log), so the sanitisation tests assert `wire_message()` is the code's fixed text — `internal` "the server could not complete the request", `incompatible_store` "the store on disk uses a format this server cannot open". `axum`'s `CatchPanicLayer` is dropped (FRE-6a: a panic is a process death with the exit code a supervisor sees, not a body), and `ConvertError`/`ReduceError`/`SketchError` gained `#[non_exhaustive]`.* - [x] → **X-NAME** `aspect_name::encode` and `validate_new` at HTTP declare (absorbs FRE-7a, SEC-0) — *partly on main: `validate` and the declare check (#66), `encode` (#72). #91 is the rest: `weftdb::aspect_name::validate_new` (`weftdb/src/types/aspect_name.rs`) runs `validate`'s chain and then refuses an ASCII uppercase letter with a new `AspectNameReason::UppercaseAsciiLetter` (the enum is `#[non_exhaustive]`, so the variant is not a breaking change). Order is asserted: the capital rule runs **after** `reason()`, so `../Price` is refused for escaping the directory, not for its capital — the more dangerous reason is never hidden. The hole is real because the legacy frame file is `-.weftseg`, which writes the name literally, and NTFS, APFS (by default) and ext4 with `casefold` fold ASCII case: two aspects differing only in case share a file, whichever seal lands second overwrites the other's bytes, and reads serve one series' values for the other's name. `encode` (#72) removes that collision for the frames D-S10 will write; this closes the declaration of a new collision. Applied at **creation only** — `POST /api/v1/storage/aspects` — so `SegmentStore::declare`, every read, seal, maintenance and store-wide sweep keep plain `validate` and a store that already holds `Price` stays readable, writable and compactable (the test asserts both halves: the refusal, then a raw catalog declare of `Price` followed by a 201 write). Two things are deliberately **not** refused: the characters only Windows forbids (`< > : " | ? *`), because `job:rate5m` is blessed on Unix by this module's own docs and is a shape in use, and the cross-platform hazard belongs one layer down where `encode` escapes `:` for write-once frame names (D-S10, the slice the plan names for it); and Unicode NFC/NFD aliases, which need `unicode-normalization` this crate does not carry, while `encode` escapes every non-ASCII byte anyway. A refusal keeps the #90 shape: `400 invalid_aspect_name` with `details.aspect` and `details.reason`.* - - [ ] **ROB-2** Storage-engine panic fail-stop: a turso-origin panic aborts + - [x] → **ROB-2** Storage-engine panic fail-stop: a turso-origin panic aborts — *#92: the write scope was already here — `weftdb::exec::control_plane_write`, and every `IndexTxn` operation runs inside it, which weftdb's own `every_op_runs_inside_the_control_plane_write_scope` asserts — so C12's "hook and knob" was the half that was missing, and it lives in the binary: `weft_server::panic_hook::install()` chains the hook it replaces, and when `in_control_plane_write()` says the panicking task held the scope it names the panic and calls `std::process::abort()`: SIGABRT for a supervisor, not the poison path's exit 70, and never an HTTP response (FRE-6a). Startup calls it immediately after `init_tracing`, because `set_hook` hands back the hook it replaced. It deliberately does not write the poison marker: that region reads before it writes (`txn.plans()`, `SegmentIndex::open`), and the marker survives the panic until an operator deletes a file. Nothing is caught, so ROB-3 stays the only catch layer and read-path panics still answer 500; the legacy `weftdb/src/types/aspect.rs` lane is ROB-5's. Proved by `weft-server/tests/control_plane_panic.rs`, which re-executes the test binary the way `poison_exit.rs` does.* - [ ] **PAC-3** Library crate hygiene: no global allocator or wgpu features imposed on library users - [ ] L-FMT: **X-FRAME** Frame registry, v7 envelope, `read_frame*`/`describe_frame`, `FrameFault` classes (absorbs FRE-4, TAG-2) - [ ] Legacy lane: **FRE-14** Legacy data-directory marker, numbered migrations and measurement upgrade test