diff --git a/src/js/bun/ffi.ts b/src/js/bun/ffi.ts index 3c885ccf1368..b406273a7d89 100644 --- a/src/js/bun/ffi.ts +++ b/src/js/bun/ffi.ts @@ -69,7 +69,7 @@ var ffi = globalThis.Bun.FFI; const ptr = (arg1, arg2) => (typeof arg2 === "undefined" ? ffi.ptr(arg1) : ffi.ptr(arg1, arg2)); const toBuffer = ffi.toBuffer; const toArrayBuffer = ffi.toArrayBuffer; -const viewSource = ffi.viewSource; +const nativeViewSource = ffi.viewSource; const BunCString = ffi.CString; const nativeLinkSymbols = ffi.linkSymbols; @@ -81,7 +81,9 @@ delete ffi.closeCallback; class JSCallback { constructor(cb, options) { - const { ctx, ptr } = nativeCallback(options, cb); + const result = nativeCallback(options, cb); + if (Error.isError(result)) throw result; + const { ctx, ptr } = result; this.#ctx = ctx; this.ptr = ptr; this.#threadsafe = !!options?.threadsafe; @@ -521,6 +523,12 @@ function cc(options) { return result; } +function viewSource(symbols, isCallback?) { + const result = nativeViewSource(symbols, isCallback); + if (Error.isError(result)) throw result; + return result; +} + function linkSymbols(options) { const result = nativeLinkSymbols(options); if (Error.isError(result)) throw result; 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 a875ab7e3499..48ea21ad60a5 100644 --- a/test/js/bun/ffi/ffi-viewSource-non-object.test.ts +++ b/test/js/bun/ffi/ffi-viewSource-non-object.test.ts @@ -1,16 +1,81 @@ +import { JSCallback, viewSource } from "bun:ffi"; import { describe, expect, test } from "bun:test"; import { isArm64, isWindows } from "harness"; const isFFIUnavailable = isWindows && isArm64; +// Captures what a call throws, or undefined if it returned normally. Written +// explicitly so the assertions below distinguish a thrown Error from a +// returned one regardless of toThrow()'s handling of returned Errors. +function thrown(fn: () => unknown): unknown { + try { + fn(); + } catch (e) { + return e; + } + return undefined; +} + describe.skipIf(isFFIUnavailable)("FFI viewSource", () => { - test("rejects non-object symbol descriptor values", () => { - // These should throw a TypeError because each symbol descriptor - // must be an object like { args: [...], returns: "void" }. - // Previously, non-object values like numbers or strings would - // cause a debug assertion failure (crash) in generateSymbolForFunction. - expect(() => Bun.FFI.viewSource({ myFunc: 42 })).toThrow("Expected an object"); - expect(() => Bun.FFI.viewSource({ myFunc: "not_an_object" })).toThrow("Expected an object"); - expect(() => Bun.FFI.viewSource({ myFunc: true })).toThrow("Expected an object"); + // Descriptor values must be objects like { args: [...], returns: "void" }. + // https://github.com/oven-sh/bun/pull/28361, https://github.com/oven-sh/bun/pull/34396 + test.each([42, "not_an_object", true])("throws on non-object symbol descriptor value %p", value => { + const err = thrown(() => viewSource({ myFunc: value as any })); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("Expected an object"); + }); + + test("throws on an unknown FFI type", () => { + const err = thrown(() => viewSource({ foo: { args: ["bogus_type" as any], returns: "void" } })); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("bogus_type"); + }); + + test.each([null, undefined, 42])("throws on non-object options argument %p", value => { + const err = thrown(() => viewSource(value as any)); + expect(err).toBeInstanceOf(TypeError); + }); + + test.each([null, undefined, 42, "str"])("throws on non-object callback descriptor %p", value => { + const err = thrown(() => viewSource(value as any, true)); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("Expected an object"); + }); + + test("returns the generated source for a valid descriptor", () => { + const src = viewSource({ foo: { args: ["i32"], returns: "i32" } }); + expect(src).toBeArray(); + expect(src).toHaveLength(1); + expect(src[0]).toContain("JSFunctionCall"); + + const cbSrc = viewSource({ args: ["i32"], returns: "i32" }, true); + expect(typeof cbSrc).toBe("string"); + expect(cbSrc).toContain("my_callback_function"); + }); +}); + +describe.skipIf(isFFIUnavailable)("FFI JSCallback", () => { + test.each([null, undefined, 42, "str", true])("throws on non-object options %p", value => { + const err = thrown(() => new JSCallback(() => {}, value as any)); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("Expected object"); + }); + + test.each([null, undefined, 42, "str", {}])("throws on non-callable callback %p", value => { + const err = thrown(() => new JSCallback(value as any, { returns: "void" })); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("Expected callback function"); + }); + + test("throws on an unknown FFI type", () => { + const err = thrown(() => new JSCallback(() => {}, { args: ["bogus_type" as any], returns: "void" })); + expect(err).toBeInstanceOf(TypeError); + expect((err as TypeError).message).toContain("bogus_type"); + }); + + test("constructs with a valid descriptor", () => { + using cb = new JSCallback(() => {}, { args: ["i32"], returns: "void" }); + expect(typeof cb.ptr).toBe("number"); + expect(cb.ptr).not.toBe(0); }); });