Skip to content

Commit e00fc79

Browse files
Unblock pampa wasm tests: feature-gate JS bridge, set wasm32 panic strategy
The raw_module extern blocks in quarto-system-runtime/src/wasm.rs import absolute paths (/src/wasm-js-bridge/*.js) that hub-client serves through Vite at runtime. wasm-bindgen generates unconditional require() calls for these paths in the shim it produces, so any wasm32 binary that links quarto-system-runtime — including the pampa wasm_lua tests — fails to load under Node.js with MODULE_NOT_FOUND before any test body runs. Add a js-bridge Cargo feature, default off. Gate the extern blocks behind it, and provide stubs that return Err("js-bridge feature not enabled") / false when off so the SystemRuntime impl still compiles. wasm-quarto-hub-client opts in via features = ["js-bridge"]; pampa's wasm test build does not, so the require()s disappear from the shim. Native builds are unaffected (wasm.rs is #![cfg(target_arch = "wasm32")]). That exposed a downstream panic-strategy problem: rust_lua_throw panics propagated as wasm RuntimeError instead of being caught by rust_lua_protected_call. Fix by mirroring wasm-quarto-hub-client's rustflags (-C panic=unwind, +bulk-memory,+exception-handling, -Zwasm-c-abi=spec) in the workspace-root .cargo/config.toml for wasm32 builds, and moving the LuaThrow marker type into wasm-c-shim so the production build and the tests share it. Update design/plan docs and dev-docs/wasm.md accordingly.
1 parent 78d717d commit e00fc79

9 files changed

Lines changed: 305 additions & 11 deletions

File tree

.cargo/config.toml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,14 @@ create-worktree = "xtask create-worktree"
99

1010
[target.wasm32-unknown-unknown]
1111
runner = "wasm-bindgen-test-runner"
12+
# Mirror the rustflags used by wasm-quarto-hub-client/.cargo/config.toml so any
13+
# wasm32 build invoked from the workspace root (e.g. the pampa wasm_lua test)
14+
# gets the same panic strategy and ABI. Without panic=unwind plus the
15+
# exception-handling target feature, wasm-c-shim's catch_unwind-based
16+
# replacement for Lua's setjmp/longjmp can't catch panics, and any Lua throw
17+
# during mlua initialization aborts the wasm module.
18+
rustflags = [
19+
"-C", "target-feature=+bulk-memory,+exception-handling",
20+
"-C", "panic=unwind",
21+
"-Zwasm-c-abi=spec",
22+
]

claude-notes/designs/2026-04-03-wasm-testing-and-cleanup.md

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,91 @@ extern crate wasm_c_shim;
413413
registers synthetic io/os, but only integration tests verify they actually work
414414
when called from Lua during filter execution on real wasm32.
415415

416+
## JS bridge isolation and panic strategy (2026-04-17)
417+
418+
After Phase 4 landed, the `wasm-tests` CI job failed at the
419+
`wasm-bindgen-test-runner` step (Node.js execution) before any test body
420+
ran, with `MODULE_NOT_FOUND` for `/src/wasm-js-bridge/cache.js`. Two
421+
distinct issues were uncovered, both originating in production code that
422+
had never previously been exercised on the wasm32 test path.
423+
424+
### Finding 1: `raw_module` extern blocks load unconditionally
425+
426+
`crates/quarto-system-runtime/src/wasm.rs` declares four
427+
`#[wasm_bindgen(raw_module = "/src/wasm-js-bridge/{template,sass,cache,fetch}.js")]`
428+
extern blocks. Hub-client serves these JS files at runtime through Vite,
429+
and `wasm-bindgen` generates `require()` calls for each one in the JS
430+
shim it produces. The `require()` happens at module-load time regardless
431+
of whether the test ever calls into JavaScript, so any wasm32 binary
432+
that links `quarto-system-runtime` cannot load under Node.js — the
433+
absolute paths don't resolve on disk.
434+
435+
The pampa wasm tests pull in `quarto-system-runtime` transitively
436+
(`WasmRuntime`, `LuaShortcodeEngine`), so the failure surfaced as soon
437+
as Phase 3 tests reached the runner step. Production never tripped this
438+
because the only wasm32 consumer (`wasm-quarto-hub-client`) runs in a
439+
browser where Vite resolves the paths.
440+
441+
**Fix:** add a `js-bridge` Cargo feature to `quarto-system-runtime`,
442+
default off. Gate the four extern blocks behind the feature, and provide
443+
stub modules that return `Err(JsValue::from_str("js-bridge feature not enabled"))`
444+
or `false` when off so the `SystemRuntime` impl still compiles.
445+
`wasm-quarto-hub-client/Cargo.toml` opts in via
446+
`features = ["js-bridge"]`. Pampa's wasm test build does not, so the
447+
`require()` calls disappear from the generated shim.
448+
449+
### Finding 2: workspace wasm32 builds inherit `panic = "abort"`
450+
451+
With Finding 1 resolved, the test runner loaded the module successfully
452+
and reached the test bodies — at which point all six tests failed in
453+
`wasm-c-shim::rust_lua_throw` with the wasm error `RuntimeError: unreachable`.
454+
455+
The `wasm-c-shim` crate replaces Lua's `setjmp`/`longjmp` with
456+
`panic!()`/`catch_unwind` (since wasm32 has no native unwinding). For
457+
this substitution to work, the binary's panic strategy must be `unwind`,
458+
not the wasm32 default of `abort`. Under `panic=abort`, `panic!()`
459+
lowers directly to the wasm `unreachable` instruction and `catch_unwind`
460+
becomes a compile-time no-op that always returns `Ok` — so the first
461+
Lua throw during mlua's protected init aborts the module.
462+
463+
The CI command's `-Zbuild-std=std,panic_unwind,panic_abort` only ensures
464+
the unwind runtime is *available* in std; it does not change the
465+
binary's panic strategy. Three additional flags are required:
466+
`-C panic=unwind`, `-C target-feature=+exception-handling`, and
467+
`-Zwasm-c-abi=spec`. `wasm-quarto-hub-client/.cargo/config.toml` already
468+
sets all three, but that config lives in an isolated workspace and never
469+
reaches builds invoked from the workspace root. Production never tripped
470+
this because hub-client builds always use the local config.
471+
472+
**Fix:** mirror the rustflags into the workspace-root `.cargo/config.toml`
473+
under `[target.wasm32-unknown-unknown]`. `[unstable] build-std` is
474+
deliberately *not* added to the workspace config — it is not target-scoped,
475+
so adding it would force `build-std` for every native invocation from the
476+
root. The `-Zbuild-std` flag stays on the test command (and in CI).
477+
478+
### Finding 3: `LuaThrow` marker placement (rebase artifact)
479+
480+
While this branch was open, main landed `Suppress noisy 'lua error' panic
481+
stack traces in WASM console` (commit `675c22d2`), which introduced a
482+
`LuaThrow` marker struct in `wasm-quarto-hub-client/src/lib.rs` and
483+
changed `rust_lua_throw` from `panic!("lua error")` to
484+
`std::panic::panic_any(crate::LuaThrow)`. Hub-client's panic hook
485+
downcasts to `LuaThrow` to filter expected control-flow panics out of
486+
console.error.
487+
488+
When this branch's "Extract C stdlib shims into shared wasm-c-shim crate"
489+
commit was rebased onto that change, `crate::LuaThrow` no longer
490+
resolved — the shim moved out of `wasm-quarto-hub-client`, so `crate::`
491+
points elsewhere.
492+
493+
**Fix:** the marker belongs in `wasm-c-shim` (where the panic
494+
originates), not in the hub-client (where the hook lives). Moved the
495+
`pub struct LuaThrow;` definition into `crates/wasm-c-shim/src/lib.rs`
496+
and have `wasm-quarto-hub-client/src/lib.rs` import it via
497+
`use wasm_c_shim::LuaThrow;`. Behavior is preserved; the dependency
498+
direction is now consistent (hub-client depends on the shim, not
499+
vice-versa).
500+
416501
## Out of scope
417502

418503
- Migrating wasm-pack usage (no longer needed — only stale crate used it)

claude-notes/plans/2026-04-07-wasm-testing-and-cleanup.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,3 +1144,74 @@ br sync --flush-only
11441144
git add .beads/
11451145
git commit -m "Sync beads: close bd-itj9 WASM testing and cleanup"
11461146
```
1147+
1148+
---
1149+
1150+
## Phase 7: Post-CI follow-up findings (2026-04-17)
1151+
1152+
After Phase 4's CI job started executing, two production-side issues
1153+
surfaced in the test path that did not affect the production wasm32
1154+
build. Both fixes landed on this branch; design rationale in the design
1155+
spec section "JS bridge isolation and panic strategy (2026-04-17)".
1156+
1157+
### Task 27: Feature-gate JS bridge in `quarto-system-runtime`
1158+
1159+
- [x] Add `js-bridge` Cargo feature (default off) to
1160+
`crates/quarto-system-runtime/Cargo.toml`.
1161+
- [x] Gate the four `#[wasm_bindgen(raw_module = ...)]` extern blocks in
1162+
`crates/quarto-system-runtime/src/wasm.rs` behind
1163+
`#[cfg(feature = "js-bridge")]`.
1164+
- [x] Provide stub modules under `#[cfg(not(feature = "js-bridge"))]`
1165+
that return `Err(JsValue::from_str("js-bridge feature not enabled"))`
1166+
or `false`, matching the original signatures so the `SystemRuntime`
1167+
impl still compiles.
1168+
- [x] Opt in from `crates/wasm-quarto-hub-client/Cargo.toml`:
1169+
`quarto-system-runtime = { ..., features = ["js-bridge"] }`.
1170+
1171+
Without this gate, `wasm-bindgen-test-runner` (Node.js) fails to load
1172+
the module with `MODULE_NOT_FOUND` for `/src/wasm-js-bridge/cache.js`,
1173+
since the absolute paths only resolve under Vite.
1174+
1175+
### Task 28: Set `panic=unwind` for workspace wasm32 builds
1176+
1177+
- [x] Add to `[target.wasm32-unknown-unknown]` in workspace
1178+
`.cargo/config.toml`:
1179+
```toml
1180+
rustflags = [
1181+
"-C", "target-feature=+bulk-memory,+exception-handling",
1182+
"-C", "panic=unwind",
1183+
"-Zwasm-c-abi=spec",
1184+
]
1185+
```
1186+
- [x] Do **not** add `[unstable] build-std` to the workspace config —
1187+
the `[unstable]` table is not target-scoped, so it would force
1188+
build-std on every native invocation. `-Zbuild-std` stays on the test
1189+
command and in CI.
1190+
1191+
Without these flags the binary inherits the wasm32 default of
1192+
`panic=abort`, which makes `wasm-c-shim::rust_lua_protected_call`'s
1193+
`catch_unwind` a no-op. The first Lua throw during mlua initialization
1194+
then aborts the wasm module rather than being caught.
1195+
1196+
### Task 29: Move `LuaThrow` into `wasm-c-shim` (rebase resolution)
1197+
1198+
- [x] Move `pub struct LuaThrow;` from
1199+
`crates/wasm-quarto-hub-client/src/lib.rs` into
1200+
`crates/wasm-c-shim/src/lib.rs` (where the panic actually originates).
1201+
- [x] Update `crates/wasm-quarto-hub-client/src/lib.rs` to
1202+
`use wasm_c_shim::LuaThrow;` for its panic-suppression hook.
1203+
1204+
The marker struct was introduced on main while this branch was open
1205+
(`Suppress noisy 'lua error' panic stack traces in WASM console`). After
1206+
rebase, `crate::LuaThrow` in the extracted shim no longer resolved.
1207+
1208+
### Task 30: Local test verification
1209+
1210+
- [x] All 6 `pampa wasm_lua` tests pass on `wasm32-unknown-unknown`
1211+
via `wasm-bindgen-test-runner` / Node.js.
1212+
- [x] `cargo check --workspace` clean (native).
1213+
- [x] `cargo check --target wasm32-unknown-unknown` clean for
1214+
`wasm-quarto-hub-client`.
1215+
1216+
Note: `cargo xtask verify` was not re-run locally — these changes
1217+
should be exercised by the existing `wasm-tests` CI job once pushed.

crates/quarto-system-runtime/Cargo.toml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ license.workspace = true
77
repository.workspace = true
88
description = "Runtime abstraction layer for Quarto system operations"
99

10+
[features]
11+
# Enables wasm-bindgen `raw_module` imports of the JS bridge files at
12+
# /src/wasm-js-bridge/{template,sass,cache,fetch}.js.
13+
#
14+
# Hub-client provides those JS modules at runtime via Vite. Off-by-default
15+
# because non-hub-client wasm32 binaries (e.g. wasm-bindgen-test runs of
16+
# pampa) have no bridge to require — Node.js fails to resolve the absolute
17+
# paths at module load. With the feature off, the impls return errors.
18+
default = []
19+
js-bridge = []
20+
1021
[dependencies]
1122
# Async methods in traits
1223
async-trait.workspace = true

crates/quarto-system-runtime/src/wasm.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use crate::vfs::{VirtualFileSystem, not_found_error};
3939
// The functions are expected to be provided via a module at the path specified.
4040
// In hub-client, this is at: /src/wasm-js-bridge/sass.js
4141

42+
#[cfg(feature = "js-bridge")]
4243
#[wasm_bindgen(raw_module = "/src/wasm-js-bridge/sass.js")]
4344
extern "C" {
4445
/// Check if SASS compilation is available.
@@ -69,6 +70,27 @@ extern "C" {
6970
) -> Result<JsValue, JsValue>;
7071
}
7172

73+
#[cfg(not(feature = "js-bridge"))]
74+
mod sass_stubs {
75+
use wasm_bindgen::prelude::*;
76+
pub(super) fn js_sass_available_impl() -> bool {
77+
false
78+
}
79+
#[allow(dead_code)]
80+
pub(super) fn js_sass_compiler_name_impl() -> String {
81+
"unavailable (js-bridge feature not enabled)".to_string()
82+
}
83+
pub(super) fn js_compile_sass_impl(
84+
_scss: &str,
85+
_style: &str,
86+
_load_paths_json: &str,
87+
) -> Result<JsValue, JsValue> {
88+
Err(JsValue::from_str("js-bridge feature not enabled"))
89+
}
90+
}
91+
#[cfg(not(feature = "js-bridge"))]
92+
use sass_stubs::{js_compile_sass_impl, js_sass_available_impl, js_sass_compiler_name_impl};
93+
7294
// =============================================================================
7395
// JavaScript Interop for Cache Operations
7496
// =============================================================================
@@ -79,6 +101,7 @@ extern "C" {
79101
// The functions are expected to be provided via a module at the path specified.
80102
// In hub-client, this is at: /src/wasm-js-bridge/cache.js
81103

104+
#[cfg(feature = "js-bridge")]
82105
#[wasm_bindgen(raw_module = "/src/wasm-js-bridge/cache.js")]
83106
extern "C" {
84107
/// Get a cached value by namespace and key.
@@ -110,6 +133,31 @@ extern "C" {
110133
fn js_cache_clear_namespace_impl(namespace: &str) -> Result<JsValue, JsValue>;
111134
}
112135

136+
#[cfg(not(feature = "js-bridge"))]
137+
mod cache_stubs {
138+
use wasm_bindgen::prelude::*;
139+
pub(super) fn js_cache_get_impl(_namespace: &str, _key: &str) -> Result<JsValue, JsValue> {
140+
Err(JsValue::from_str("js-bridge feature not enabled"))
141+
}
142+
pub(super) fn js_cache_set_impl(
143+
_namespace: &str,
144+
_key: &str,
145+
_value: &js_sys::Uint8Array,
146+
) -> Result<JsValue, JsValue> {
147+
Err(JsValue::from_str("js-bridge feature not enabled"))
148+
}
149+
pub(super) fn js_cache_delete_impl(_namespace: &str, _key: &str) -> Result<JsValue, JsValue> {
150+
Err(JsValue::from_str("js-bridge feature not enabled"))
151+
}
152+
pub(super) fn js_cache_clear_namespace_impl(_namespace: &str) -> Result<JsValue, JsValue> {
153+
Err(JsValue::from_str("js-bridge feature not enabled"))
154+
}
155+
}
156+
#[cfg(not(feature = "js-bridge"))]
157+
use cache_stubs::{
158+
js_cache_clear_namespace_impl, js_cache_delete_impl, js_cache_get_impl, js_cache_set_impl,
159+
};
160+
113161
// =============================================================================
114162
// JavaScript Interop for Network Fetch
115163
// =============================================================================
@@ -121,6 +169,7 @@ extern "C" {
121169
// Binary content is base64-encoded by the JS side to avoid complex type
122170
// marshalling. The Rust side decodes it with the base64 crate.
123171

172+
#[cfg(feature = "js-bridge")]
124173
#[wasm_bindgen(raw_module = "/src/wasm-js-bridge/fetch.js")]
125174
extern "C" {
126175
/// Fetch content from a URL.
@@ -133,6 +182,11 @@ extern "C" {
133182
fn js_fetch_url_impl(url: &str) -> Result<JsValue, JsValue>;
134183
}
135184

185+
#[cfg(not(feature = "js-bridge"))]
186+
fn js_fetch_url_impl(_url: &str) -> Result<JsValue, JsValue> {
187+
Err(JsValue::from_str("js-bridge feature not enabled"))
188+
}
189+
136190
// =============================================================================
137191
// Monotonic clock: performance.now()
138192
// =============================================================================

crates/wasm-c-shim/src/lib.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,12 @@
2424

2525
#[cfg(target_arch = "wasm32")]
2626
mod shim;
27+
28+
/// Marker payload for panics raised by `rust_lua_throw` (the WASM replacement
29+
/// for Lua's `LUAI_THROW`). Each Lua error inside `lua_pcall` produces one
30+
/// such panic, which `rust_lua_protected_call` catches microseconds later.
31+
///
32+
/// Hosts that install a custom panic hook (e.g. wasm-quarto-hub-client) can
33+
/// downcast to this type to filter expected Lua control-flow panics out of
34+
/// console.error logs without suppressing real Rust panics.
35+
pub struct LuaThrow;

crates/wasm-quarto-hub-client/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ quarto-lsp-core = { path = "../quarto-lsp-core" }
2727
quarto-pandoc-types = { path = "../quarto-pandoc-types" }
2828
quarto-sass = { path = "../quarto-sass" }
2929
quarto-source-map = "0.1.0"
30-
quarto-system-runtime = { path = "../quarto-system-runtime" }
30+
quarto-system-runtime = { path = "../quarto-system-runtime", features = ["js-bridge"] }
3131
quarto-project-create = { path = "../quarto-project-create" }
3232
quarto-trace = { path = "../quarto-trace" }
3333
flate2 = "1.1"

crates/wasm-quarto-hub-client/src/lib.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,7 @@
1212
#[cfg(target_arch = "wasm32")]
1313
extern crate wasm_c_shim;
1414

15-
/// Sentinel panic payload raised by `c_shim::rust_lua_throw`.
16-
///
17-
/// On wasm32 Lua's `LUAI_THROW` macro cannot use `setjmp`/`longjmp`, so
18-
/// it is rewired to raise a Rust panic that `rust_lua_protected_call`
19-
/// catches via `catch_unwind`. This happens on every Lua runtime error —
20-
/// including ones caught by `pcall` — so the panic is expected control
21-
/// flow. The `init()` panic hook filters panics carrying this payload
22-
/// so they do not spam `console.error` with stack traces.
23-
pub struct LuaThrow;
15+
use wasm_c_shim::LuaThrow;
2416

2517
use std::cell::RefCell;
2618
use std::path::Path;

0 commit comments

Comments
 (0)