fix: keep the thrown value identifiable in the bun and deno error wrappers

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jb7qAQ3vnyhsbPZk7Qi7ho

backend/tests/bun_jobs.rs

@@ -364,6 +364,81 @@ export function main() {
     Ok(())
 }
 
+/// A script can throw a non-object, or an object with a reference cycle. The error the
+/// job reports must still identify what was thrown, never the wrapper's own failure to
+/// read or serialize it.
+#[sqlx::test(fixtures("base"))]
+async fn test_bun_job_non_error_throws(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    fn bun_job(content: &str) -> JobPayload {
+        JobPayload::Code(RawCode {
+            hash: None,
+            content: content.to_owned(),
+            path: None,
+            language: ScriptLang::Bun,
+            lock: None,
+            concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default(
+            )
+            .into(),
+            debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(),
+            cache_ttl: None,
+            cache_ignore_s3_path: None,
+            dedicated_worker: None,
+            modules: None,
+            tag: None,
+        })
+    }
+
+    // `throw null`: the wrapper must not dereference the thrown value.
+    {
+        let job = bun_job("export function main() { throw null; }");
+        let completed = run_job_in_new_worker_until_complete(&db, false, job, port).await;
+        assert!(!completed.success);
+        let result = completed.json_result().unwrap();
+        assert_eq!(result["error"]["message"], serde_json::json!("null"));
+    }
+
+    // Circular object: several HTTP clients throw errors that link two objects
+    // both ways, which a plain JSON.stringify cannot serialize.
+    {
+        let job = bun_job(
+            r#"
+export function main() {
+    const a: any = { name: "circular", message: "boom" };
+    a.self = a;
+    throw a;
+}
+"#,
+        );
+        let completed = run_job_in_new_worker_until_complete(&db, false, job, port).await;
+        assert!(!completed.success);
+        let result = completed.json_result().unwrap();
+        let error = &result["error"];
+        assert_eq!(error["message"], serde_json::json!("boom"));
+        assert_eq!(
+            error["extra"]["self"]["self"],
+            serde_json::json!("[Circular]")
+        );
+    }
+
+    // A thrown string is a string, not an index map of its characters.
+    {
+        let job = bun_job(r#"export function main() { throw "nur ein string"; }"#);
+        let completed = run_job_in_new_worker_until_complete(&db, false, job, port).await;
+        assert!(!completed.success);
+        let result = completed.json_result().unwrap();
+        assert_eq!(
+            result["error"]["message"],
+            serde_json::json!("nur ein string")
+        );
+    }
+
+    Ok(())
+}
+
 #[sqlx::test(fixtures("base"))]
 async fn test_bun_job_missing_main_function(db: Pool<Postgres>) -> anyhow::Result<()> {
     initialize_tracing().await;

backend/tests/worker.rs

@@ -3240,6 +3240,75 @@ export async function main(a: Date) {
     Ok(())
 }
 
+/// A script can throw a non-object. The error the job reports must still identify what
+/// was thrown, never the wrapper's own failure to read it.
+#[sqlx::test(fixtures("base"))]
+async fn test_deno_job_non_error_throws(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    let deno_job = |content: &str| {
+        JobPayload::Code(RawCode {
+            hash: None,
+            content: content.to_owned(),
+            path: None,
+            lock: None,
+            language: ScriptLang::Deno,
+            cache_ttl: None,
+            cache_ignore_s3_path: None,
+            dedicated_worker: None,
+            concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default(
+            )
+            .into(),
+            debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(),
+            modules: None,
+            tag: None,
+        })
+    };
+
+    // `throw null`: the wrapper must not dereference the thrown value.
+    let job = RunJob::from(deno_job("export function main() { throw null; }"))
+        .run_until_complete(&db, false, port)
+        .await;
+    assert!(!job.success);
+    assert_eq!(
+        job.json_result().unwrap()["error"]["message"],
+        json!("null")
+    );
+
+    // A thrown string reaches the result as a string; reading `message` off it yields
+    // nothing, which used to leave an empty error object.
+    let job = RunJob::from(deno_job(
+        r#"export function main() { throw "nur ein string"; }"#,
+    ))
+    .run_until_complete(&db, false, port)
+    .await;
+    assert!(!job.success);
+    assert_eq!(
+        job.json_result().unwrap()["error"]["message"],
+        json!("nur ein string")
+    );
+
+    // Deno reports only message/name/stack. Collecting the thrown value's own properties
+    // would persist whatever an HTTP client hangs off its errors, auth headers included.
+    let job = RunJob::from(deno_job(
+        r#"
+export function main() {
+    throw Object.assign(new Error("boom"), { authorization: "Bearer secret" });
+}
+"#,
+    ))
+    .run_until_complete(&db, false, port)
+    .await;
+    assert!(!job.success);
+    let error = job.json_result().unwrap()["error"].clone();
+    assert_eq!(error["message"], json!("boom"));
+    assert!(error.get("extra").is_none(), "unexpected extra: {error}");
+
+    Ok(())
+}
+
 /// Test that full .npmrc content works for deno jobs with private registries.
 /// Requires:
 /// - `TEST_NPMRC` environment variable set to the full .npmrc content

backend/windmill-worker/src/bun_executor.rs

@@ -22,7 +22,7 @@ use crate::{
         build_command_with_isolation, create_args_and_out_file, get_reserved_variables,
         parse_npm_config, read_file, read_file_content, read_result, resolve_nsjail_timeout,
         resolve_nsjail_tmp_mount_block, start_child_process, write_file_binary, MaybeLock,
-        OccupancyMetrics, StreamNotifier, DEV_CONF_NSJAIL,
+        OccupancyMetrics, StreamNotifier, DEV_CONF_NSJAIL, JS_ERROR_SERIALIZER,
     },
     get_proxy_envs_for_lang,
     handle_child::handle_child,
@@ -235,6 +235,7 @@ import * as Readline from "node:readline"
 BigInt.prototype.toJSON = function () {{
     return this.toString();
 }};
+{JS_ERROR_SERIALIZER}
 
 const scripts = new Map();
 {functions}
@@ -266,7 +267,7 @@ for await (const line of Readline.createInterface({{ input: process.stdin }})) {
             const res = await entry.module.main(...mainArgs);
             console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value));
         }} catch (e) {{
-            console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: argsJson }}));
+            console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: argsJson }})));
         }}
         continue;
     }}
@@ -280,7 +281,7 @@ for await (const line of Readline.createInterface({{ input: process.stdin }})) {
             const res = await entry.module.main(...args);
             console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value));
         }} catch (e) {{
-            console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: argsJson }}));
+            console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: argsJson }})));
         }}
         continue;
     }}
@@ -314,7 +315,7 @@ for await (const line of Readline.createInterface({{ input: process.stdin }})) {
             const res = await entry.module.main(...mainArgs);
             console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value));
         }} catch (e) {{
-            console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: argsJson }}));
+            console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: argsJson }})));
         }}
         continue;
     }}
@@ -340,7 +341,7 @@ for await (const line of Readline.createInterface({{ input: process.stdin }})) {
             const res = await entry.module.main(...args);
             console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value));
         }} catch (e) {{
-            console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: argsJson }}));
+            console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: argsJson }})));
         }}
         continue;
     }}
@@ -1912,6 +1913,7 @@ function argsObjToArr({{ {spread} }}) {{
 BigInt.prototype.toJSON = function () {{
     return this.toString();
 }};
+{JS_ERROR_SERIALIZER}
 
 // Find the workflow entrypoint (export default)
 let workflowFn = Main.default;
@@ -1990,22 +1992,12 @@ try {{
     process.exit(0);
 }} catch(e) {{
     console.error(e);
-    let err = {{ message: e.message, name: e.name, stack: e.stack }};
-    let step_id = process.env.WM_FLOW_STEP_ID;
+    const err = wmToErrorObject(e, true);
+    const step_id = process.env.WM_FLOW_STEP_ID;
     if (step_id) {{
         err["step_id"] = step_id;
     }}
-    const extra = {{}};
-    Object.getOwnPropertyNames(e).forEach((key) => {{
-        if (['line', 'name', 'stack', 'column', 'message', 'sourceURL', 'originalLine', 'originalColumn'].includes(key)) {{
-            return;
-        }}
-        extra[key] = e[key];
-    }});
-    if (Object.keys(extra).length > 0) {{
-        err["extra"] = extra;
-    }}
-    await fs.writeFile("result.json", JSON.stringify(err));
+    await fs.writeFile("result.json", wmSerializeError(err));
     process.exit(1);
 }}
     "#,
@@ -2030,6 +2022,7 @@ function isAsyncIterable(obj) {{
 BigInt.prototype.toJSON = function () {{
     return this.toString();
 }};
+{JS_ERROR_SERIALIZER}
 
 async function run() {{
     {dates}
@@ -2056,22 +2049,12 @@ try {{
     await run();
 }} catch(e) {{
     console.error(e);
-    let err = {{ message: e.message, name: e.name, stack: e.stack }};
-    let step_id = process.env.WM_FLOW_STEP_ID;
+    const err = wmToErrorObject(e, true);
+    const step_id = process.env.WM_FLOW_STEP_ID;
     if (step_id) {{
         err["step_id"] = step_id;
     }}
-    const extra = {{}};
-    Object.getOwnPropertyNames(e).forEach((key) => {{
-        if (['line', 'name', 'stack', 'column', 'message', 'sourceURL', 'originalLine', 'originalColumn'].includes(key)) {{
-            return;
-        }}
-        extra[key] = e[key];
-    }});
-    if (Object.keys(extra).length > 0) {{
-        err["extra"] = extra;
-    }}
-    await fs.writeFile("result.json", JSON.stringify(err));
+    await fs.writeFile("result.json", wmSerializeError(err));
     process.exit(1);
 }}
     "#,

backend/windmill-worker/src/common.rs

@@ -67,6 +67,86 @@ mount {
 #[cfg(not(debug_assertions))]
 pub const DEV_CONF_NSJAIL: &str = "";
 
+/// Helpers the Bun/Deno wrappers use to turn a thrown value into the error object
+/// they report, injected into the generated wrapper source.
+///
+/// A script can throw anything, so neither reading `e.message` nor calling
+/// `JSON.stringify` on the result is safe: `throw null`, and errors that link two
+/// objects both ways (an axios error hangs `response` off `request` and back), both
+/// abort the wrapper itself, and the job then reports a stack trace from the wrapper
+/// instead of what the script threw.
+///
+/// `collectExtra` is true only where the wrapper already reported the thrown value's own
+/// properties, which is the Bun main and Bun WAC v2 wrappers alone. Turning it on for the
+/// other three would newly persist whatever an HTTP client hangs off its errors,
+/// credential-bearing headers included, into the run history of every existing script on
+/// those paths; that parity is its own decision, not part of stopping a crash.
+///
+/// Kept comment-free, since it is written out per job.
+pub const JS_ERROR_SERIALIZER: &str = r#"
+const wmErrOwnKeys = ['line', 'name', 'stack', 'column', 'message', 'sourceURL', 'originalLine', 'originalColumn'];
+
+function wmErrString(v) {
+    if (typeof v === 'string') return v;
+    try {
+        return String(v);
+    } catch (_) {
+        return '[unstringifiable ' + typeof v + ']';
+    }
+}
+
+function wmErrProp(e, key) {
+    try {
+        return e[key];
+    } catch (_) {
+        return undefined;
+    }
+}
+
+function wmToErrorObject(e, collectExtra) {
+    if (e === null || typeof e !== 'object') {
+        return { message: wmErrString(e), name: 'ThrownValue' };
+    }
+    const err = {};
+    for (const field of ['message', 'name', 'stack']) {
+        const v = wmErrProp(e, field);
+        if (v != null) err[field] = wmErrString(v);
+    }
+    if (collectExtra) {
+        let keys = [];
+        try {
+            keys = Object.getOwnPropertyNames(e);
+        } catch (_) {}
+        const extra = {};
+        for (const key of keys) {
+            if (wmErrOwnKeys.includes(key)) continue;
+            extra[key] = wmErrProp(e, key);
+        }
+        if (Object.keys(extra).length > 0) err.extra = extra;
+    }
+    if (Object.keys(err).length === 0) {
+        err.message = wmErrString(e);
+        err.name = 'ThrownValue';
+    }
+    return err;
+}
+
+function wmSerializeError(err) {
+    const ancestors = [];
+    try {
+        return JSON.stringify(err, function (key, v) {
+            if (v === null || typeof v !== 'object') return v;
+            while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop();
+            if (ancestors.indexOf(v) !== -1) return '[Circular]';
+            ancestors.push(v);
+            return v;
+        });
+    } catch (_) {
+        return JSON.stringify({ message: err.message, name: err.name, stack: err.stack, step_id: err.step_id, line: err.line });
+    }
+}
+"#;
+
 /// Turn a JSON value into the string a shell/CLI arg should receive: a JSON string
 /// becomes its inner value, anything else is re-serialized compactly.
 pub(crate) fn raw_to_string(x: &str) -> String {

backend/windmill-worker/src/deno_executor.rs

@@ -9,7 +9,7 @@ use crate::{
     common::{
         build_command_with_isolation, create_args_and_out_file, get_reserved_variables,
         parse_npm_config, read_file, read_result, start_child_process, OccupancyMetrics,
-        StreamNotifier,
+        StreamNotifier, JS_ERROR_SERIALIZER,
     },
     get_proxy_envs_for_lang,
     handle_child::handle_child,
@@ -342,6 +342,7 @@ function argsObjToArr({{ {spread} }}) {{
 BigInt.prototype.toJSON = function () {{
     return this.toString();
 }};
+{JS_ERROR_SERIALIZER}
 
 function isAsyncIterable(obj) {{
     return obj != null && typeof obj[Symbol.asyncIterator] === 'function';
@@ -368,12 +369,12 @@ async function run() {{
 try {{
     await run();
 }} catch(e) {{
-    let err = {{ message: e.message, name: e.name, stack: e.stack }};
-    let step_id = Deno.env.get("WM_FLOW_STEP_ID");
+    const err = wmToErrorObject(e, false);
+    const step_id = Deno.env.get("WM_FLOW_STEP_ID");
     if (step_id) {{
         err["step_id"] = step_id;
     }}
-    await Deno.writeTextFile("result.json", JSON.stringify(err));
+    await Deno.writeTextFile("result.json", wmSerializeError(err));
     Deno.exit(1);
 }}
     "#,
@@ -662,7 +663,7 @@ pub fn generate_dedicated_worker_wrapper(inner_content: &str) -> Result<String>
                 let res: any = await main(...[ {spread} ]);
                 console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value) + '\n');
             }} catch (e) {{
-                console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: line }}) + '\n');
+                console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: line }})) + '\n');
             }}
             continue;
         }}"#
@@ -679,6 +680,7 @@ import {{ main }} from "./main.ts";
 BigInt.prototype.toJSON = function () {{
     return this.toString();
 }};
+{JS_ERROR_SERIALIZER}
 
 console.log('start\n');
 
@@ -705,7 +707,7 @@ for await (const chunk of Deno.stdin.readable) {{
                 let res: any = await main(...[ {spread} ]);
                 console.log("wm_res[success]:" + JSON.stringify(res ?? null, (key, value) => typeof value === 'undefined' ? null : value) + '\n');
             }} catch (e) {{
-                console.log("wm_res[error]:" + JSON.stringify({{ message: e.message, name: e.name, stack: e.stack, line: argsJson }}) + '\n');
+                console.log("wm_res[error]:" + wmSerializeError(Object.assign(wmToErrorObject(e, false), {{ line: argsJson }})) + '\n');
             }}
             continue;
         }}