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
7 changes: 5 additions & 2 deletions src/runtime/ffi/ffi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,10 @@ impl FFI {
Some(ZigString::init(b"Out of memory").to_error_instance(global_this))
})
{
return Ok(val);
// The JSCallback constructor destructures `{ ctx, ptr }` from this
// return, so a returned error is silently dropped (`ptr === undefined`).
// Throw it instead, the way dlopen() does.
return Err(global_this.throw_value(val));
}

// TODO: WeakRefHandle that automatically frees it?
Expand Down Expand Up @@ -1840,7 +1843,7 @@ pub(super) fn generate_symbol_for_function(
));
}

if function.threadsafe && return_type != ABIType::Void {
if threadsafe && return_type != ABIType::Void {
return Ok(Some(
ZigString::static_(b"Threadsafe functions must return void").to_error_instance(global),
));
Expand Down
18 changes: 18 additions & 0 deletions test/js/bun/ffi/ffi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,24 @@ function ffiRunner(fast) {
await 1;
});
}

it("rejects non-void return types", () => {
// threadsafe callbacks are delivered asynchronously and can't return a value to native code
expect(
() =>
new JSCallback(() => 1, {
args: ["int"],
returns: "int",
threadsafe: true,
}),
).toThrow("Threadsafe functions must return void");
});
});

it("JSCallback surfaces construction errors instead of returning ptr === undefined", () => {
// A native validation error was returned as a value and silently dropped by the
// constructor's { ctx, ptr } destructure (ptr === undefined); it now throws.
expect(() => new JSCallback(() => 1, { args: ["int"], returns: "notarealtype" })).toThrow("Unknown return type");
});

describe("integer identities work for all possible values", () => {
Expand Down