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]) +}