From 38bba84c1c80429fb11d2f45d42a8d21e447db39 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:43:37 +0700 Subject: [PATCH] Bun and Deno in CI, and the stack a native error was missing One binary serves Node, Bun and Deno, because N-API is the ABI all three implement. What made that a claim rather than a fact is that only Node ran anything, so the whole suite now runs on Bun 1.3 and Deno 2.9 as well, and both found something. Bun found a native error with no `stack` at all. V8 writes one when the error is made and JavaScriptCore writes none through N-API, so on Bun `err.stack` was `undefined` and every logger reaching for it printed that word instead of the failure. The header line is what the other runtimes have there, since an error made under a running statement has no JavaScript frames beneath it anyway, so this writes that line when it finds none. Defined rather than assigned, because the one V8 writes is not enumerable and a plain assignment would have put `stack` in `Object.keys(err)` on one runtime and not the other three, which is a difference every structured logger would print. Deno needed `--allow-read --allow-write --allow-env --allow-ffi`, which is now the list the job runs with rather than a `-A` that proves nothing, and `--no-check` for the checkout: the tests reach the declarations by relative path here rather than through `npm:zudb`, and Deno reads the `.cjs` beside them as a module with an `export =` in it. The package job is what checks the types, through the resolution a user actually gets. `npm run test:bun` and `npm run test:deno` run the same suite locally. Node 24 also runs it a second time with `--harmony-temporal`, and 26 has Temporal unflagged, so all four runs are green: 90 tests, 88 of them wherever Temporal exists. --- .github/workflows/ci.yml | 46 ++++++++++++++++++++++++++++++++++++++++ README.md | 8 ++++--- package.json | 2 ++ src/error.rs | 45 +++++++++++++++++++++++++++++++-------- 4 files changed, 89 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97b5e5a..2523b9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,3 +90,49 @@ jobs: # one V8 no longer has. - run: npm run test:temporal if: matrix.node == 24 + + # Bun and Deno run the same suite over the same binary, because the + # binary is the same one: N-API is the ABI all three implement, and a + # runtime this package claims and nothing runs on is a runtime this + # package has broken. Node builds the addon in both jobs, since the + # build is napi-rs and cargo and neither has anything to do with which + # runtime is going to load what they made. + bun: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 + with: + node-version: 24 + - uses: oven-sh/setup-bun@v2 + with: + bun-version: 1.3.14 + - uses: Swatinem/rust-cache@v2 + - run: npm ci + - run: npm run build:debug + # `bun test` and not `bun run`, because the tests are written + # against `node:test` and Bun's shim for it refuses to register a + # test outside its own runner. + - run: npm run test:bun + + deno: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 + with: + node-version: 24 + - uses: denoland/setup-deno@v2 + with: + deno-version: 2.9.5 + - uses: Swatinem/rust-cache@v2 + - run: npm ci + - run: npm run build:debug + # Permissions rather than `-A`, so the list a Deno user has to + # write is a list this job has proved is enough. `--no-check` is + # about the checkout and not about the package: the tests reach + # the declarations through relative paths here rather than through + # `npm:zudb`, and Deno reads the `.cjs` beside them as a module + # with an `export =` in it. The package job above is what checks + # the types, through the resolution a user actually gets. + - run: npm run test:deno diff --git a/README.md b/README.md index 610581d..7a4e447 100644 --- a/README.md +++ b/README.md @@ -170,14 +170,16 @@ Bun and Deno in CI, and the WASM build for the browser. | Runtime | How | Notes | |---|---|---| -| Node >= 24 (LTS) | prebuilt N-API binary | CI runs 24 and 26 | -| Bun >= 1.3 | the same binary | tested as a first-class target, not assumed | -| Deno >= 2.9 | the same binary via `npm:zudb`, or `jsr:@zu/zudb` | needs `--allow-ffi --allow-read` | +| Node >= 24 (LTS) | prebuilt N-API binary | CI runs 24 and 26, and 24 again with `--harmony-temporal` | +| Bun >= 1.3 | the same binary | CI runs the whole suite on it | +| Deno >= 2.9 | the same binary via `npm:zudb`, or `jsr:@zu/zudb` | CI runs the whole suite on it, with `--allow-read --allow-write --allow-env --allow-ffi` | | Browser and edge | `zudb/wasm` | read-mostly, over OPFS or HTTP range requests | | Electron | the same binary | N-API is ABI-stable across Electron versions, so no per-Electron rebuild | `apache-arrow` is an optional peer dependency behind the `zudb/arrow` entry point, so the base package stays small. +One binary serves all three, because N-API is the ABI all three implement, and the whole suite runs on each of them in CI rather than the other two being assumed from Node passing. `npm run test:bun` and `npm run test:deno` run it locally. What the three do not agree on is what a native error carries: V8 writes a `stack` when the error is made and JavaScriptCore writes none at all through N-API, so this client writes the header line itself when it finds none, non-enumerably, and `err.stack` starts with the condition's name on all of them. + ## Specification Spec/2064g/dx/05-typescript.md in [tamnd/zu](https://github.com/tamnd/zu). Milestone: DX3 (tamnd/zu#169). diff --git a/package.json b/package.json index be708a4..13dc067 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,8 @@ "build:debug": "napi build --platform --js binding.cjs --dts binding.d.cts", "test": "node --test \"test/*.test.mjs\"", "test:temporal": "node --harmony-temporal --test \"test/*.test.mjs\"", + "test:bun": "bun test test/", + "test:deno": "deno test --no-check --allow-read --allow-write --allow-env --allow-ffi \"test/*.test.mjs\"", "check:types": "tsc --noEmit --project test/types/tsconfig.json", "check:package": "attw --pack .", "bench": "node bench/query.mjs", diff --git a/src/error.rs b/src/error.rs index 611f3fe..250d7bd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -16,7 +16,7 @@ //! narrows the `unknown` a `catch` gives you. use napi::bindgen_prelude::*; -use napi::{Env, Status}; +use napi::{Env, Status, ValueType}; use zudb::ZuError; /// Builds the exception for `err` and wraps it as the failure to @@ -41,8 +41,7 @@ pub fn raise(env: &Env, err: ZuError) -> Error { } fn build<'env>(env: &'env Env, err: &ZuError) -> Result> { - let mut object = blank(env, &err.to_string())?; - object.set("name", name_for(err))?; + let mut object = blank(env, name_for(err), &err.to_string())?; object.set("retryable", err.retryable())?; if let Some(status) = err.gqlstatus() { object.set("code", status.code())?; @@ -122,8 +121,7 @@ pub fn usage(env: &Env, message: impl AsRef) -> Error { } fn usage_object<'env>(env: &'env Env, message: &str) -> Result> { - let mut object = blank(env, message)?; - object.set("name", "ZuUsageError")?; + let mut object = blank(env, "ZuUsageError", message)?; object.set("retryable", false)?; Ok(object) } @@ -142,21 +140,50 @@ pub fn aborted(env: &Env, message: &str) -> Error { } fn aborted_object<'env>(env: &'env Env, message: &str) -> Result> { - let mut object = blank(env, message)?; - object.set("name", "AbortError")?; + let mut object = blank(env, "AbortError", message)?; object.set("retryable", false)?; Ok(object) } -/// An `Error` with `message` and nothing else on it yet. +/// An `Error` with a name, a message, a stack and nothing else on it yet. /// /// napi writes the status it was handed into `code`, and `code` here is /// the GQLSTATUS. A caller reading `err.code === '42001'` cannot be /// given `GenericFailure` for a condition that has no code of its own, /// so the one napi wrote is removed and the real one, when there is /// one, is written in its place. -fn blank<'env>(env: &'env Env, message: &str) -> Result> { +fn blank<'env>(env: &'env Env, name: &str, message: &str) -> Result> { let mut object = env.create_error(Error::new(Status::GenericFailure, message.to_string()))?; object.delete_named_property("code")?; + object.set("name", name)?; + stacked(env, &mut object, name, message)?; Ok(object) } + +/// Gives the error a `stack` on a runtime that gave it none. +/// +/// V8 writes one when the error is made, and for one made under a +/// statement it is the header line and nothing else, since there are no +/// JavaScript frames beneath a native call that is already running. +/// JavaScriptCore writes no `stack` at all through napi, so on Bun +/// `err.stack` is `undefined` and every logger that reaches for it +/// prints that word instead of the failure. The header line is what the +/// other runtimes have and it is what this writes, after the name, so +/// the first line says `ZuSyntaxError` on all four of them. An error +/// that arrived with a stack keeps the one it has, frames and all. +fn stacked(env: &Env, object: &mut Object<'_>, name: &str, message: &str) -> Result<()> { + let existing: Unknown = object.get_named_property_unchecked("stack")?; + if existing.get_type()? == ValueType::String { + return Ok(()); + } + // Defined rather than assigned, because the one V8 writes is not + // enumerable and this one has to match: a plain assignment puts + // `stack` in `Object.keys(err)`, and then every structured logger + // that walks an error's own keys prints the trace as a field on one + // runtime and not on the other three. + let property = Property::new() + .with_utf8_name("stack")? + .with_napi_value(env, format!("{name}: {message}"))? + .with_property_attributes(PropertyAttributes::Writable | PropertyAttributes::Configurable); + object.define_properties(&[property]) +}