Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions src/runtime/ffi/ffi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)? {
Comment thread
robobun marked this conversation as resolved.
return Ok(val);
}

Expand Down Expand Up @@ -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<JSValue> {
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)? {
Comment thread
robobun marked this conversation as resolved.
return Ok(val);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

let mut arraylist: Vec<u8> = Vec::new();
Expand All @@ -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(
Expand All @@ -1372,7 +1366,7 @@ impl FFI {
) -> JsResult<JSValue> {
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);
}
}

Expand All @@ -1386,9 +1380,7 @@ impl FFI {
let mut symbols = StringArrayHashMap::<Function>::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)? {
Comment thread
robobun marked this conversation as resolved.
// an error while validating symbols
// keys/arg_types freed by Drop
return Ok(val);
Expand Down
53 changes: 53 additions & 0 deletions test/js/bun/ffi/ffi-viewSource-non-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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");
Expand Down
Loading