diff --git a/src/runtime/ffi/ffi_body.rs b/src/runtime/ffi/ffi_body.rs index 0027fad813d5..b5073499bbed 100644 --- a/src/runtime/ffi/ffi_body.rs +++ b/src/runtime/ffi/ffi_body.rs @@ -1275,11 +1275,7 @@ impl FFI { let mut function = Function::default(); let func = &mut function; - if let Some(val) = generate_symbol_for_function(global_this, interface, func) - .unwrap_or_else(|_| { - Some(ZigString::init(b"Out of memory").to_error_instance(global_this)) - }) - { + if let Some(val) = generate_symbol_for_function(global_this, interface, func)? { return Ok(val); } @@ -1338,18 +1334,16 @@ impl FFI { Ok(JSValue::UNDEFINED) } - pub fn print_callback(global: &JSGlobalObject, object: JSValue) -> JSValue { + pub fn print_callback(global: &JSGlobalObject, object: JSValue) -> JsResult { jsc::mark_binding(); if object.is_empty_or_undefined_or_null() || !object.is_object() { - return global.to_invalid_arguments(format_args!("Expected an object")); + return Ok(global.to_invalid_arguments(format_args!("Expected an object"))); } let mut function = Function::default(); - if let Some(val) = generate_symbol_for_function(global, object, &mut function) - .unwrap_or_else(|_| Some(ZigString::init(b"Out of memory").to_error_instance(global))) - { - return val; + if let Some(val) = generate_symbol_for_function(global, object, &mut function)? { + return Ok(val); } let mut arraylist: Vec = Vec::new(); @@ -1360,9 +1354,9 @@ impl FFI { .print_callback_source_code(None, None, &mut arraylist) .is_err() { - return ZigString::init(b"Error while printing code").to_error_instance(global); + return Ok(ZigString::init(b"Error while printing code").to_error_instance(global)); } - jsc::bun_string_jsc::create_utf8_for_js(global, &arraylist).unwrap_or(JSValue::ZERO) + jsc::bun_string_jsc::create_utf8_for_js(global, &arraylist) } pub fn print( @@ -1372,7 +1366,7 @@ impl FFI { ) -> JsResult { if let Some(is_callback) = is_callback_val { if is_callback.to_boolean() { - return Ok(Self::print_callback(global, object)); + return Self::print_callback(global, object); } } @@ -1386,9 +1380,7 @@ impl FFI { let mut symbols = StringArrayHashMap::::default(); // SAFETY: `get_object()` returned a non-null `*mut JSObject`; `object` keeps it alive. let obj = unsafe { &*obj }; - if let Some(val) = - generate_symbols(global, &mut symbols, obj).unwrap_or(Some(JSValue::ZERO)) - { + if let Some(val) = generate_symbols(global, &mut symbols, obj)? { // an error while validating symbols // keys/arg_types freed by Drop return Ok(val); diff --git a/test/js/bun/ffi/ffi-viewSource-non-object.test.ts b/test/js/bun/ffi/ffi-viewSource-non-object.test.ts index 48ea21ad60a5..5d7e13844dc2 100644 --- a/test/js/bun/ffi/ffi-viewSource-non-object.test.ts +++ b/test/js/bun/ffi/ffi-viewSource-non-object.test.ts @@ -42,6 +42,43 @@ describe.skipIf(isFFIUnavailable)("FFI viewSource", () => { expect((err as TypeError).message).toContain("Expected an object"); }); + // print_callback swallowed a pending exception from the descriptor's getters + // and returned a non-empty "Out of memory" error instance instead, tripping + // "host fn return/exception state mismatch". The getter's error must propagate. + test.each(["args", "threadsafe", "returns", "ptr"])( + "propagates a throwing %s getter in the callback descriptor", + prop => { + const message = `boom from ${prop} getter`; + const err = thrown(() => + viewSource( + { + get [prop]() { + throw new Error(message); + }, + }, + true, + ), + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe(message); + }, + ); + + // Sibling path through generate_symbols (the non-callback form of viewSource). + test("propagates a throwing getter in a symbol descriptor", () => { + const err = thrown(() => + viewSource({ + sym: { + get args() { + throw new Error("boom from symbol args getter"); + }, + }, + }), + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe("boom from symbol args getter"); + }); + test("returns the generated source for a valid descriptor", () => { const src = viewSource({ foo: { args: ["i32"], returns: "i32" } }); expect(src).toBeArray(); @@ -73,6 +110,22 @@ describe.skipIf(isFFIUnavailable)("FFI JSCallback", () => { expect((err as TypeError).message).toContain("bogus_type"); }); + // FFI::callback had the same bug as print_callback: a descriptor getter that + // throws left the exception pending while callback() returned an error value. + test.each(["args", "threadsafe", "returns", "ptr"])("propagates a throwing %s getter in the descriptor", prop => { + const message = `boom from ${prop} getter`; + const err = thrown( + () => + new JSCallback(() => {}, { + get [prop]() { + throw new Error(message); + }, + }), + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe(message); + }); + test("constructs with a valid descriptor", () => { using cb = new JSCallback(() => {}, { args: ["i32"], returns: "void" }); expect(typeof cb.ptr).toBe("number");