node errors: render a single class name in ERR_INVALID_ARG_TYPE as "an instance of" like node

src/jsc/bindings/ErrorCode.cpp

@@ -641,34 +641,52 @@ void addParameter(WTF::StringBuilder& result, const StringView& arg_name)
     }
 }
 
+static bool isPrimitiveTypeName(StringView type);
+static bool isClassName(StringView type);
+
+// Node treats a string `expected` as a one-entry list, so a single entry is
+// classified the same way the list overload below classifies each of its
+// entries: kTypes names render "of type x", class names "an instance of X".
+// Callers of this overload also pass pre-rendered lists ("string or an
+// instance of Buffer"), which Node never sees; those keep the verbatim
+// "of type ..." rendering.
+// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L1404
 WTF::String ERR_INVALID_ARG_TYPE(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, const StringView& arg_name, const StringView& expected_type, JSValue actual_value)
 {
     WTF::StringBuilder result;
     result.append("The "_s);
     addParameter(result, arg_name);
     result.append(" must be "_s);
 
-    // Node categorizes a free-form phrase like "Array of unique strings"
-    // (spaces, but not a flattened "X, Y, or Z" list) as neither a primitive
-    // type name nor a class name: it renders "must be an Array of unique
-    // strings", not "must be of type ...". Flattened lists keep the legacy
-    // "of type" rendering.
-    bool isPhrase = expected_type.contains(' ') && !expected_type.contains(", "_s) && !expected_type.contains(" or "_s);
-    if (isPhrase) {
-        bool hasUppercase = false;
-        for (unsigned i = 0; i < expected_type.length(); i++) {
-            if (isASCIIUpper(expected_type[i])) {
-                hasUppercase = true;
-                break;
+    if (isPrimitiveTypeName(expected_type)) {
+        result.append("of type "_s);
+        result.append(expected_type.convertToASCIILowercase());
+    } else if (isClassName(expected_type)) {
+        result.append("an instance of "_s);
+        result.append(expected_type);
+    } else {
+        // Node categorizes a free-form phrase like "Array of unique strings"
+        // (spaces, but not a flattened "X, Y, or Z" list) as neither a primitive
+        // type name nor a class name: it renders "must be an Array of unique
+        // strings", not "must be of type ...". Flattened lists keep the legacy
+        // "of type" rendering.
+        bool isPhrase = expected_type.contains(' ') && !expected_type.contains(", "_s) && !expected_type.contains(" or "_s);
+        if (isPhrase) {
+            bool hasUppercase = false;
+            for (unsigned i = 0; i < expected_type.length(); i++) {
+                if (isASCIIUpper(expected_type[i])) {
+                    hasUppercase = true;
+                    break;
+                }
             }
+            if (hasUppercase)
+                result.append("an "_s);
+        } else {
+            result.append("of type "_s);
         }
-        if (hasUppercase)
-            result.append("an "_s);
-    } else {
-        result.append("of type "_s);
+        result.append(expected_type);
     }
 
-    result.append(expected_type);
     result.append(". Received "_s);
     determineSpecificType(JSC::getVM(globalObject), globalObject, result, actual_value);
     RETURN_IF_EXCEPTION(scope, {});
@@ -677,15 +695,15 @@ WTF::String ERR_INVALID_ARG_TYPE(JSC::ThrowScope& scope, JSC::JSGlobalObject* gl
 
 // Matches Node's kTypes list: primitive type names accepted by ERR_INVALID_ARG_TYPE.
 // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L72
-static bool isPrimitiveTypeName(const WTF::String& type)
+static bool isPrimitiveTypeName(StringView type)
 {
     return type == "string"_s || type == "function"_s || type == "number"_s
         || type == "object"_s || type == "Function"_s || type == "Object"_s
         || type == "boolean"_s || type == "bigint"_s || type == "symbol"_s;
 }
 
 // Matches Node's classRegExp /^[A-Z][a-zA-Z0-9]*$/.
-static bool isClassName(const WTF::String& type)
+static bool isClassName(StringView type)
 {
     if (type.isEmpty() || !isASCIIUpper(type[0]))
         return false;
@@ -889,7 +907,9 @@ JSC::EncodedJSValue INVALID_ARG_TYPE_INSTANCE(JSC::ThrowScope& throwScope, JSC::
     return {};
 }
 
-// When you want INVALID_ARG_TYPE to say "The argument must be an instance of X. Received Y." instead of "The argument must be of type X. Received Y."
+// Renders "must be an instance of <expected_type>" verbatim, so it also takes a
+// pre-rendered list like "Buffer, TypedArray, or DataView". INVALID_ARG_TYPE
+// produces the same text when given a single class name.
 JSC::EncodedJSValue INVALID_ARG_INSTANCE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value)
 {
     auto& vm = JSC::getVM(globalObject);

src/jsc/bindings/ErrorCode.h

@@ -86,6 +86,10 @@ enum Bound {
 namespace ERR {
 
 JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, ASCIILiteral message);
+// `expected_type` is classified like a one-entry Node list: a primitive type name renders
+// "must be of type string", a class name "must be an instance of KeyObject", a phrase
+// "must be an Array of unique strings". A pre-rendered list such as "string or an instance of
+// Buffer" is printed as written after "must be of type ".
 JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value);
 JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, JSC::JSValue arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value);
 // Renders Node's multi-type list ("must be one of type number or string").

src/jsc/bindings/JSNodePerformanceHooksHistogramPrototype.cpp

@@ -119,7 +119,7 @@ JSC_DEFINE_HOST_FUNCTION(jsNodePerformanceHooksHistogramProtoFuncAdd, (JSGlobalO
     JSValue otherArg = callFrame->uncheckedArgument(0);
     JSNodePerformanceHooksHistogram* otherHistogram = dynamicDowncast<JSNodePerformanceHooksHistogram>(otherArg);
     if (!otherHistogram) [[unlikely]] {
-        Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "argument"_s, "Histogram"_s, otherArg);
+        Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "other"_s, "RecordableHistogram"_s, otherArg);
         return {};
     }
 

src/jsc/bindings/JSX509CertificatePrototype.cpp

@@ -328,10 +328,10 @@ JSC_DEFINE_HOST_FUNCTION(jsX509CertificateProtoFuncCheckIssued, (JSGlobalObject
     if (!thisObject) [[unlikely]]
         return throwVMError(globalObject, scope, createError(globalObject, ErrorCode::ERR_INVALID_THIS, "checkIssued called on incompatible receiver"_s));
 
-    JSX509Certificate* issuer = dynamicDowncast<JSX509Certificate>(callFrame->argument(0));
+    JSValue otherCertValue = callFrame->argument(0);
+    JSX509Certificate* issuer = dynamicDowncast<JSX509Certificate>(otherCertValue);
     if (!issuer) {
-        Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_ARG_TYPE, "issuer must be a JSX509Certificate"_s);
-        return {};
+        return ERR::INVALID_ARG_TYPE(scope, globalObject, "otherCert"_s, "X509Certificate"_s, otherCertValue);
     }
 
     auto check = thisObject->checkIssued(globalObject, issuer);

test/js/bun/perf_hooks/histogram.test.ts

@@ -99,6 +99,23 @@ describe("Histogram", () => {
     assert.strictEqual(h1.max, 4);
   });
 
+  // Messages verified against node v26.3.0.
+  test("add() rejects a non-histogram with Node's argument name and class name", () => {
+    const h = createHistogram();
+
+    assert.throws(() => h.add(1), {
+      name: "TypeError",
+      code: "ERR_INVALID_ARG_TYPE",
+      message: 'The "other" argument must be an instance of RecordableHistogram. Received type number (1)',
+    });
+    assert.throws(() => h.add({}), {
+      name: "TypeError",
+      code: "ERR_INVALID_ARG_TYPE",
+      message: 'The "other" argument must be an instance of RecordableHistogram. Received an instance of Object',
+    });
+    assert.strictEqual(h.count, 0);
+  });
+
   test("reset functionality", () => {
     const h = createHistogram();
 

test/js/node/crypto/x509-subclass.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { X509Certificate } from "node:crypto";
+import { createPrivateKey, createPublicKey, X509Certificate } from "node:crypto";
 import { readFileSync } from "node:fs";
 import path from "node:path";
 
@@ -124,8 +124,56 @@ describe("X509Certificate#checkIssued", () => {
     expect(result).toBe(false);
   });
 
+  // Messages verified against node v26.3.0.
   test("throws ERR_INVALID_ARG_TYPE when the argument is not an X509Certificate", () => {
-    expect(() => agent1.checkIssued({} as any)).toThrow();
-    expect(() => agent1.checkIssued("" as any)).toThrow();
+    expect(() => agent1.checkIssued({} as any)).toThrow(
+      expect.objectContaining({
+        name: "TypeError",
+        code: "ERR_INVALID_ARG_TYPE",
+        message: 'The "otherCert" argument must be an instance of X509Certificate. Received an instance of Object',
+      }),
+    );
+    expect(() => agent1.checkIssued("" as any)).toThrow(
+      expect.objectContaining({
+        name: "TypeError",
+        code: "ERR_INVALID_ARG_TYPE",
+        message: `The "otherCert" argument must be an instance of X509Certificate. Received type string ('')`,
+      }),
+    );
+  });
+});
+
+// Messages verified against node v26.3.0.
+describe("X509Certificate#checkPrivateKey and #verify argument validation", () => {
+  const agent1 = new X509Certificate(certPem);
+  const privateKey = createPrivateKey(readFileSync(path.join(keysDir, "agent1-key.pem")));
+  const publicKey = createPublicKey(privateKey);
+
+  test.each([
+    [1, "type number (1)"],
+    [{}, "an instance of Object"],
+    [undefined, "undefined"],
+    [null, "null"],
+    ["pem", "type string ('pem')"],
+  ])("%p is rejected with an instance-of KeyObject message", (value, received) => {
+    const expected = expect.objectContaining({
+      name: "TypeError",
+      code: "ERR_INVALID_ARG_TYPE",
+      message: `The "pkey" argument must be an instance of KeyObject. Received ${received}`,
+    });
+    expect(() => agent1.checkPrivateKey(value as any)).toThrow(expected);
+    expect(() => agent1.verify(value as any)).toThrow(expected);
+  });
+
+  test("a KeyObject of the wrong type is still ERR_INVALID_ARG_VALUE", () => {
+    expect(() => agent1.checkPrivateKey(publicKey)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" }));
+    expect(() => agent1.verify(privateKey)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" }));
+  });
+
+  test("matching keys are accepted", () => {
+    expect(agent1.checkPrivateKey(privateKey)).toBe(true);
+    // agent1 is signed by ca1, not by its own key.
+    expect(agent1.verify(publicKey)).toBe(false);
+    expect(agent1.verify(new X509Certificate(ca1Pem).publicKey)).toBe(true);
   });
 });

test/js/node/errors/invalid-arg-type-expected.test.ts

@@ -0,0 +1,150 @@
+import { describe, expect, test } from "bun:test";
+import { createHash } from "node:crypto";
+import { EventEmitter, addAbortListener, on, once } from "node:events";
+import { Session } from "node:inspector";
+import { timerify } from "node:perf_hooks";
+import { Duplex, Readable, Writable, addAbortSignal } from "node:stream";
+import { SyntheticModule } from "node:vm";
+
+// The "must be ..." half of an ERR_INVALID_ARG_TYPE message is rendered by
+// Message::ERR_INVALID_ARG_TYPE in src/jsc/bindings/ErrorCode.cpp. Node's
+// lib/internal/errors.js treats a string `expected` as a one-entry list and
+// classifies the entry: a kTypes name renders "of type x" (lower-cased), a name
+// matching /^[A-Z][a-zA-Z0-9]*$/ renders "an instance of X", anything else is
+// printed as written. The same formatter is reached from C++ callers that pass a
+// C++ argument name, from C++ validators that receive the name from JS, and from
+// the builtins' $ERR_INVALID_ARG_TYPE(name, "X", value). Every message below is
+// what node v26.3.0 prints for the same call.
+
+function caught(fn: () => unknown) {
+  try {
+    fn();
+  } catch (error: any) {
+    return { name: error.name, code: error.code, message: error.message };
+  }
+  throw new Error("expected the call to throw");
+}
+
+type Case = [label: string, call: () => unknown, message: string];
+
+describe("ERR_INVALID_ARG_TYPE with a single class name renders 'an instance of'", () => {
+  const cases: Case[] = [
+    // C++ caller with a C++ argument name (JSBuffer.cpp copyBytesFrom).
+    [
+      "Buffer.copyBytesFrom(1)",
+      () => Buffer.copyBytesFrom(1 as any),
+      'The "view" argument must be an instance of TypedArray. Received type number (1)',
+    ],
+    [
+      "Buffer.copyBytesFrom({})",
+      () => Buffer.copyBytesFrom({} as any),
+      'The "view" argument must be an instance of TypedArray. Received an instance of Object',
+    ],
+
+    // C++ validator whose argument name comes from JS (NodeValidator.cpp
+    // validateAbortSignal); a dotted name is a "property".
+    [
+      "events.on() with a primitive signal",
+      () => on(new EventEmitter(), "x", { signal: 1 as any }),
+      'The "options.signal" property must be an instance of AbortSignal. Received type number (1)',
+    ],
+    [
+      "events.on() with an object that is not a signal",
+      () => on(new EventEmitter(), "x", { signal: {} as any }),
+      'The "options.signal" property must be an instance of AbortSignal. Received an instance of Object',
+    ],
+    [
+      "events.on() with a null signal",
+      () => on(new EventEmitter(), "x", { signal: null as any }),
+      'The "options.signal" property must be an instance of AbortSignal. Received null',
+    ],
+
+    // Builtins calling $ERR_INVALID_ARG_TYPE(name, "ClassName", value).
+    [
+      "events.addAbortListener(1)",
+      () => addAbortListener(1 as any, () => {}),
+      'The "signal" argument must be an instance of AbortSignal. Received type number (1)',
+    ],
+    [
+      "stream.addAbortSignal(1)",
+      () => addAbortSignal(1 as any, new Readable()),
+      'The "signal" argument must be an instance of AbortSignal. Received type number (1)',
+    ],
+    [
+      "new Readable({ signal: 'abc' })",
+      () => new Readable({ signal: "abc" as any }),
+      `The "signal" argument must be an instance of AbortSignal. Received type string ('abc')`,
+    ],
+    [
+      "Readable.fromWeb(1)",
+      () => Readable.fromWeb(1 as any),
+      'The "readableStream" argument must be an instance of ReadableStream. Received type number (1)',
+    ],
+    [
+      "Writable.fromWeb(1)",
+      () => Writable.fromWeb(1 as any),
+      'The "writableStream" argument must be an instance of WritableStream. Received type number (1)',
+    ],
+    [
+      "Duplex.fromWeb({ readable: 1 })",
+      () => Duplex.fromWeb({ readable: 1, writable: 1 } as any),
+      'The "pair.readable" property must be an instance of ReadableStream. Received type number (1)',
+    ],
+    [
+      "perf_hooks.timerify(fn, { histogram: 1 })",
+      () => timerify(() => {}, { histogram: 1 as any }),
+      'The "options.histogram" property must be an instance of RecordableHistogram. Received type number (1)',
+    ],
+  ];
+
+  test.each(cases)("%s", (_label, call, message) => {
+    expect(caught(call)).toEqual({ name: "TypeError", code: "ERR_INVALID_ARG_TYPE", message });
+  });
+
+  test("events.once(1) rejects with the same rendering", async () => {
+    const error: any = await once(1 as any, "x").then(
+      () => {
+        throw new Error("expected once() to reject");
+      },
+      error => error,
+    );
+    expect({ name: error.name, code: error.code, message: error.message }).toEqual({
+      name: "TypeError",
+      code: "ERR_INVALID_ARG_TYPE",
+      message: 'The "emitter" argument must be an instance of EventEmitter. Received type number (1)',
+    });
+  });
+});
+
+test("a kTypes name spelled with a capital renders lower-cased like node", () => {
+  // inspector.ts passes "Object"; node's validateObject prints "object".
+  expect(caught(() => new Session().post("Runtime.evaluate", 1 as any))).toEqual({
+    name: "TypeError",
+    code: "ERR_INVALID_ARG_TYPE",
+    message: 'The "params" argument must be of type object. Received type number (1)',
+  });
+});
+
+describe("renderings that do not involve a class name are unchanged", () => {
+  const cases: Case[] = [
+    [
+      "primitive type name (process.chdir)",
+      () => process.chdir(1 as any),
+      'The "directory" argument must be of type string. Received type number (1)',
+    ],
+    [
+      "pre-rendered list (Hash#update)",
+      () => createHash("sha256").update(1 as any),
+      'The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received type number (1)',
+    ],
+    [
+      "free-form phrase (vm.SyntheticModule)",
+      () => new SyntheticModule(1 as any, () => {}),
+      'The "exportNames" argument must be an Array of unique strings. Received type number (1)',
+    ],
+  ];
+
+  test.each(cases)("%s", (_label, call, message) => {
+    expect(caught(call)).toEqual({ name: "TypeError", code: "ERR_INVALID_ARG_TYPE", message });
+  });
+});