diff --git a/src/jsc/bindings/ErrorCode.cpp b/src/jsc/bindings/ErrorCode.cpp index 6114d1d771e6..70f283538912 100644 --- a/src/jsc/bindings/ErrorCode.cpp +++ b/src/jsc/bindings/ErrorCode.cpp @@ -372,6 +372,60 @@ void JSValueToStringSafe(JSC::JSGlobalObject* globalObject, WTF::StringBuilder& builder.append(Bun__inspect_singleline(globalObject, arg).transferToWTFString()); } +// Port of the `object` arm of Node's determineSpecificType: +// if (value.constructor && 'name' in value.constructor) return `an instance of ${value.constructor.name}`; +// return inspect(value, { depth: -1 }); +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L1038-L1041 +static void appendSpecificTypeOfObject(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF::StringBuilder& builder, JSC::JSObject* object) +{ + auto scope = DECLARE_THROW_SCOPE(vm); + + JSValue constructor = object->get(globalObject, vm.propertyNames->constructor); + RETURN_IF_EXCEPTION(scope, ); + if (constructor.toBoolean(globalObject)) { + JSObject* constructorObject = constructor.getObject(); + if (!constructorObject) { + // `'name' in ` throws, so node's message builder throws too (worded as V8 words it). + String description; + if (constructor.isSymbol()) + description = asSymbol(constructor)->tryGetDescriptiveString().value_or("Symbol()"_s); + else + description = constructor.toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, ); + throwTypeError(globalObject, scope, makeString("Cannot use 'in' operator to search for 'name' in "_s, description)); + return; + } + JSValue name = constructorObject->getIfPropertyExists(globalObject, vm.propertyNames->name); + RETURN_IF_EXCEPTION(scope, ); + if (name) { + auto* nameString = name.toString(globalObject); + RETURN_IF_EXCEPTION(scope, ); + auto nameView = nameString->view(globalObject); + RETURN_IF_EXCEPTION(scope, ); + builder.append("an instance of "_s); + builder.append(nameView); + return; + } + } + + JSFunction* utilInspect = defaultGlobalObject(globalObject)->utilInspectFunction(); + RETURN_IF_EXCEPTION(scope, ); + JSObject* options = constructEmptyObject(globalObject); + options->putDirect(vm, Identifier::fromString(vm, "depth"_s), jsNumber(-1), 0); + auto callData = JSC::getCallData(utilInspect); + MarkedArgumentBuffer arguments; + arguments.append(object); + arguments.append(options); + ASSERT(!arguments.hasOverflowed()); + JSValue inspected = JSC::profiledCall(globalObject, ProfilingReason::API, utilInspect, callData, jsUndefined(), arguments); + RETURN_IF_EXCEPTION(scope, ); + auto* inspectedString = inspected.toString(globalObject); + RETURN_IF_EXCEPTION(scope, ); + auto inspectedView = inspectedString->view(globalObject); + RETURN_IF_EXCEPTION(scope, ); + builder.append(inspectedView); +} + void determineSpecificType(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF::StringBuilder& builder, JSValue value) { auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); @@ -392,6 +446,9 @@ void determineSpecificType(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF:: if (d != d) return builder.append("type number (NaN)"_s); if (d == infinity) return builder.append("type number (Infinity)"_s); if (d == -infinity) return builder.append("type number (-Infinity)"_s); + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L1021-L1022 + // Number-to-string drops the sign of -0, so node special-cases it. + if (d == 0 && std::signbit(d)) return builder.append("type number (-0)"_s); builder.append("type number ("_s); builder.append(d); builder.append(')'); @@ -490,19 +547,9 @@ void determineSpecificType(JSC::VM& vm, JSC::JSGlobalObject* globalObject, WTF:: return; } if (cell->isObject()) { - auto constructor = value.get(globalObject, vm.propertyNames->constructor); - RETURN_IF_EXCEPTION(scope, void()); - if (constructor.toBoolean(globalObject)) { - auto name = constructor.get(globalObject, vm.propertyNames->name); - RETURN_IF_EXCEPTION(scope, void()); - auto str = name.toString(globalObject); - RETURN_IF_EXCEPTION(scope, void()); - builder.append("an instance of "_s); - auto view = str->view(globalObject); - RETURN_IF_EXCEPTION(scope, ); - builder.append(view); - return; - } + appendSpecificTypeOfObject(vm, globalObject, builder, cell->getObject()); + RETURN_IF_EXCEPTION(scope, ); + return; } // value = lazyInternalUtilInspect().inspect(value, { colors: false }); diff --git a/test/js/node/errors/invalid-arg-type-received.test.ts b/test/js/node/errors/invalid-arg-type-received.test.ts new file mode 100644 index 000000000000..56e341081538 --- /dev/null +++ b/test/js/node/errors/invalid-arg-type-received.test.ts @@ -0,0 +1,182 @@ +import { describe, expect, test } from "bun:test"; +import EventEmitter from "node:events"; +import { SocketAddress } from "node:net"; + +// The "Received ..." suffix of ERR_INVALID_ARG_TYPE is rendered by determineSpecificType() in +// src/jsc/bindings/ErrorCode.cpp, a port of node's lib/internal/errors.js determineSpecificType(). +// Native classes also use it for the "but received ..." suffix of ERR_INVALID_THIS. The entry +// points below reach it from a C++ validator, from the JS builtins' $ERR_INVALID_ARG_TYPE, from +// Rust's determine_specific_type, and from createInvalidThisError. Every rendering and every +// thrown message asserted here is what node v26.3.0 prints for the same value through the three +// ERR_INVALID_ARG_TYPE entry points. +const entryPoints: [label: string, code: string, prefix: string, throwWith: (value: unknown) => unknown][] = [ + [ + "C++ validator (process.chdir)", + "ERR_INVALID_ARG_TYPE", + 'The "directory" argument must be of type string. Received ', + value => process.chdir(value as any), + ], + [ + "$ERR_INVALID_ARG_TYPE (EventEmitter#on)", + "ERR_INVALID_ARG_TYPE", + 'The "listener" argument must be of type function. Received ', + value => new EventEmitter().on("event", value as any), + ], + [ + "Rust determine_specific_type (SocketAddress.parse)", + "ERR_INVALID_ARG_TYPE", + 'The "input" argument must be of type string. Received ', + value => SocketAddress.parse(value as any), + ], + [ + "createInvalidThisError (CryptoHasher#update)", + "ERR_INVALID_THIS", + "Expected this to be instanceof CryptoHasher, but received ", + value => Bun.CryptoHasher.prototype.update.call(value, "x"), + ], +]; + +const inspectCustom = Symbol.for("nodejs.util.inspect.custom"); + +const cases: [label: string, make: () => unknown, rendered: string][] = [ + // Number.prototype.toString drops the sign of -0; node special-cases it. + ["negative zero", () => -0, "type number (-0)"], + ["zero", () => 0, "type number (0)"], + ["negative fraction", () => -1.5, "type number (-1.5)"], + + // `value.constructor && 'name' in value.constructor` -> `an instance of ${value.constructor.name}` + ["plain object", () => ({}), "an instance of Object"], + ["array", () => [], "an instance of Array"], + ["instance of an anonymous class", () => new (class {})(), "an instance of "], + ["non-function constructor with a name", () => ({ constructor: { name: "Custom" } }), "an instance of Custom"], + ["constructor whose name is undefined", () => ({ constructor: { name: undefined } }), "an instance of undefined"], + [ + "constructor that inherits its name", + () => ({ constructor: Object.create({ name: "Inherited" }) }), + "an instance of Inherited", + ], + + // Otherwise node falls back to util.inspect(value, { depth: -1 }), which collapses the value to + // its constructor name in brackets instead of printing its contents. + ["constructor without a name", () => ({ constructor: {} }), "[Object]"], + ["constructor is null", () => ({ constructor: null }), "[Object]"], + ["constructor is a falsy primitive", () => ({ constructor: "" }), "[Object]"], + [ + "constructor whose has trap hides name", + () => ({ constructor: new Proxy({ name: "Hidden" }, { has: () => false }) }), + "[Object]", + ], + [ + "class instance shadowing its constructor with a nameless one", + () => Object.assign(new (class Foo {})(), { constructor: {} }), + "[Foo]", + ], + ["Map with a null constructor", () => Object.assign(new Map([[1, 2]]), { constructor: null }), "[Map]"], + ["empty null-prototype object", () => Object.create(null), "[Object: null prototype] {}"], + [ + "null-prototype object with properties", + () => Object.assign(Object.create(null), { a: 1 }), + "[Object: null prototype]", + ], + [ + "custom inspect function receives depth -1", + () => ({ constructor: null, [inspectCustom]: (depth: number) => `custom depth=${depth}` }), + "custom depth=-1", + ], +]; + +// `'name' in value.constructor` throws when the constructor is a truthy primitive, so node's +// message builder throws a plain TypeError instead of producing ERR_INVALID_ARG_TYPE. +const primitiveConstructors: [label: string, constructor: unknown, rendered: string][] = [ + ["number", 1, "1"], + ["string", "abc", "abc"], + ["boolean", true, "true"], + ["symbol", Symbol("desc"), "Symbol(desc)"], + ["bigint", 1n, "1"], +]; + +function thrownBy(fn: () => unknown): any { + try { + fn(); + } catch (e) { + return e; + } + throw new Error("expected the call to throw"); +} + +describe.each(entryPoints)("determineSpecificType via %s", (_label, code, prefix, throwWith) => { + test("renders -0 and objects like node", () => { + const messages = cases.map(([label, make]) => { + const error = thrownBy(() => throwWith(make())); + return [label, error.code, error.message]; + }); + expect(messages).toEqual(cases.map(([label, , rendered]) => [label, code, prefix + rendered])); + }); + + test("a truthy primitive constructor throws the `in` operator's TypeError", () => { + const errors = primitiveConstructors.map(([label, constructor]) => { + const error = thrownBy(() => throwWith({ constructor })); + return [label, error instanceof TypeError, error.code, error.message]; + }); + expect(errors).toEqual( + primitiveConstructors.map(([label, , rendered]) => [ + label, + true, + undefined, + `Cannot use 'in' operator to search for 'name' in ${rendered}`, + ]), + ); + }); + + test("an exception thrown while rendering the value replaces the error being built", () => { + const constructorBoom = new RangeError("constructor getter"); + const hasBoom = new RangeError("has trap"); + const nameBoom = new RangeError("name getter"); + const inspectBoom = new RangeError("custom inspect"); + + expect( + thrownBy(() => + throwWith({ + get constructor() { + throw constructorBoom; + }, + }), + ), + ).toBe(constructorBoom); + expect( + thrownBy(() => + throwWith({ + constructor: new Proxy( + {}, + { + has() { + throw hasBoom; + }, + }, + ), + }), + ), + ).toBe(hasBoom); + expect( + thrownBy(() => + throwWith({ + constructor: { + get name() { + throw nameBoom; + }, + }, + }), + ), + ).toBe(nameBoom); + expect( + thrownBy(() => + throwWith({ + constructor: null, + [inspectCustom]() { + throw inspectBoom; + }, + }), + ), + ).toBe(inspectBoom); + }); +});