From 2db9ab4c729617a649ca7acada5d31a2a961ec42 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Tue, 25 Feb 2025 15:35:14 -0800 Subject: [PATCH] synchronize all impls of ERR_INVALID_ARG_TYPE (#17158) --- src/bun.js/bindings/ErrorCode.cpp | 104 +++++++++--------- .../test-url-format-invalid-input.js | 1 - .../node/test/parallel/test-buffer-alloc.js | 2 +- .../test/parallel/test-buffer-arraybuffer.js | 5 +- .../parallel/test-buffer-compare-offset.js | 2 +- .../node/test/parallel/test-buffer-compare.js | 6 +- .../node/test/parallel/test-buffer-concat.js | 6 +- .../node/test/parallel/test-buffer-equals.js | 2 +- .../test-dns-setservers-type-check.js | 2 +- .../node/test/parallel/test-process-hrtime.js | 2 +- .../test/parallel/test-process-setgroups.js | 2 +- .../node/test/parallel/test-string-decoder.js | 2 +- 12 files changed, 71 insertions(+), 65 deletions(-) diff --git a/src/bun.js/bindings/ErrorCode.cpp b/src/bun.js/bindings/ErrorCode.cpp index 76fca95eae..f73bf4a364 100644 --- a/src/bun.js/bindings/ErrorCode.cpp +++ b/src/bun.js/bindings/ErrorCode.cpp @@ -16,6 +16,7 @@ #include "JavaScriptCore/JSType.h" #include "JavaScriptCore/Symbol.h" #include "wtf/Assertions.h" +#include "wtf/Vector.h" #include "wtf/text/ASCIIFastPath.h" #include "wtf/text/ASCIILiteral.h" #include "wtf/text/MakeString.h" @@ -464,12 +465,56 @@ extern "C" BunString Bun__ErrorCode__determineSpecificType(JSC::JSGlobalObject* namespace Message { +void addList(WTF::StringBuilder& result, WTF::Vector& types) +{ + switch (types.size()) { + case 0: + return; + case 1: + result.append(types.at(0)); + return; + case 2: + result.append(types.at(0)); + result.append(" or "_s); + result.append(types.at(1)); + return; + case 3: + result.append(types.at(0)); + result.append(", "_s); + result.append(types.at(1)); + result.append(", or "_s); + result.append(types.at(2)); + return; + default: { + for (unsigned i = 0; i < types.size() - 1; i++) { + result.append(types.at(i)); + result.append(", "_s); + } + result.append("or "_s); + result.append(types.at(types.size() - 1)); + return; + } + } +} + +void addParameter(WTF::StringBuilder& result, const StringView& arg_name) +{ + if (arg_name.endsWith(" argument"_s)) { + result.append(arg_name); + } else { + result.append("\""_s); + result.append(arg_name); + result.append("\" "_s); + result.append(arg_name.contains('.') ? "property"_s : "argument"_s); + } +} + 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); - result.append(arg_name); - result.append("\" argument must be of type "_s); + result.append("The "_s); + addParameter(result, arg_name); + result.append(" must be of type "_s); result.append(expected_type); result.append(". Received "_s); determineSpecificType(JSC::getVM(globalObject), globalObject, result, actual_value); @@ -482,15 +527,9 @@ WTF::String ERR_INVALID_ARG_TYPE(JSC::ThrowScope& scope, JSC::JSGlobalObject* gl WTF::StringBuilder result; result.append("The "_s); - - if (arg_name.endsWith(" argument"_s)) { - result.append(arg_name); - } else { - result.append("\""_s); - result.append(arg_name); - result.append("\" argument"_s); - } - result.append(" must be of type "_s); + addParameter(result, arg_name); + result.append(" must be "_s); + result.append("of type "_s); unsigned length = expected_types.size(); if (length == 1) { @@ -582,24 +621,8 @@ namespace ERR { 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) { - WTF::StringBuilder builder; - auto arg_kind = arg_name.contains('.') ? "property"_s : "argument"_s; - auto ty_first_char = expected_type[0]; - auto ty_kind = ty_first_char >= 'A' && ty_first_char <= 'Z' ? "an instance of"_s : "of type"_s; - - builder.append("The \""_s); - builder.append(arg_name); - builder.append("\" "_s); - builder.append(arg_kind); - builder.append(" must be "_s); - builder.append(ty_kind); - builder.append(" "_s); - builder.append(expected_type); - builder.append(". Received "_s); - determineSpecificType(globalObject->vm(), globalObject, builder, val_actual_value); - RETURN_IF_EXCEPTION(throwScope, {}); - - throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, builder.toString())); + auto message = Message::ERR_INVALID_ARG_TYPE(throwScope, globalObject, arg_name, expected_type, val_actual_value); + throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, message)); return {}; } @@ -609,25 +632,8 @@ JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalO RETURN_IF_EXCEPTION(throwScope, {}); auto arg_name = jsString->view(globalObject); RETURN_IF_EXCEPTION(throwScope, {}); - - WTF::StringBuilder builder; - auto arg_kind = arg_name->contains('.') ? "property"_s : "argument"_s; - auto ty_first_char = expected_type[0]; - auto ty_kind = ty_first_char >= 'A' && ty_first_char <= 'Z' ? "an instance of"_s : "of type"_s; - - builder.append("The \""_s); - builder.append(arg_name); - builder.append("\" "_s); - builder.append(arg_kind); - builder.append(" must be "_s); - builder.append(ty_kind); - builder.append(" "_s); - builder.append(expected_type); - builder.append(". Received "_s); - determineSpecificType(globalObject->vm(), globalObject, builder, val_actual_value); - RETURN_IF_EXCEPTION(throwScope, {}); - - throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, builder.toString())); + auto message = Message::ERR_INVALID_ARG_TYPE(throwScope, globalObject, arg_name, expected_type, val_actual_value); + throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, message)); return {}; } diff --git a/test/js/node/test/parallel/needs-test/test-url-format-invalid-input.js b/test/js/node/test/parallel/needs-test/test-url-format-invalid-input.js index 7ccb472a8d..451c45aed5 100644 --- a/test/js/node/test/parallel/needs-test/test-url-format-invalid-input.js +++ b/test/js/node/test/parallel/needs-test/test-url-format-invalid-input.js @@ -29,4 +29,3 @@ test('format invalid input', () => { assert.strictEqual(url.format(''), ''); assert.strictEqual(url.format({}), ''); }); - diff --git a/test/js/node/test/parallel/test-buffer-alloc.js b/test/js/node/test/parallel/test-buffer-alloc.js index 67f61f37c5..3215f1f055 100644 --- a/test/js/node/test/parallel/test-buffer-alloc.js +++ b/test/js/node/test/parallel/test-buffer-alloc.js @@ -1083,7 +1083,7 @@ assert.throws( { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "target" argument must be an instance of Buffer or ' + + message: 'The "target" argument must be of type Buffer or ' + 'Uint8Array. Received undefined' }); diff --git a/test/js/node/test/parallel/test-buffer-arraybuffer.js b/test/js/node/test/parallel/test-buffer-arraybuffer.js index d721888d15..de1b064788 100644 --- a/test/js/node/test/parallel/test-buffer-arraybuffer.js +++ b/test/js/node/test/parallel/test-buffer-arraybuffer.js @@ -43,8 +43,9 @@ assert.throws(function() { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The first argument must be of type string, Buffer, ArrayBuffer, Array, or Array-like Object.' + - ' Received an instance of AB' + message: 'The first argument must be of type string, ' + + 'Buffer, ArrayBuffer, Array, or Array-like Object. Received ' + + 'an instance of AB' }); // Test the byteOffset and length arguments diff --git a/test/js/node/test/parallel/test-buffer-compare-offset.js b/test/js/node/test/parallel/test-buffer-compare-offset.js index 9f6f733547..dfedeb654c 100644 --- a/test/js/node/test/parallel/test-buffer-compare-offset.js +++ b/test/js/node/test/parallel/test-buffer-compare-offset.js @@ -89,6 +89,6 @@ assert.throws(() => a.compare(b, -Infinity, Infinity), oor); assert.throws(() => a.compare(), { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "target" argument must be an instance of ' + + message: 'The "target" argument must be of type ' + 'Buffer or Uint8Array. Received undefined' }); diff --git a/test/js/node/test/parallel/test-buffer-compare.js b/test/js/node/test/parallel/test-buffer-compare.js index 42f9dc2c0c..8827d11c7e 100644 --- a/test/js/node/test/parallel/test-buffer-compare.js +++ b/test/js/node/test/parallel/test-buffer-compare.js @@ -30,18 +30,18 @@ assert.strictEqual(Buffer.compare(Buffer.alloc(1), Buffer.alloc(0)), 1); assert.throws(() => Buffer.compare(Buffer.alloc(1), 'abc'), { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "buf2" argument must be an instance of Buffer or Uint8Array.' + + message: 'The "buf2" argument must be of type Buffer or Uint8Array.' + common.invalidArgTypeHelper('abc') }); assert.throws(() => Buffer.compare('abc', Buffer.alloc(1)), { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "buf1" argument must be an instance of Buffer or Uint8Array.' + + message: 'The "buf1" argument must be of type Buffer or Uint8Array.' + common.invalidArgTypeHelper('abc') }); assert.throws(() => Buffer.alloc(1).compare('abc'), { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "target" argument must be an instance of ' + + message: 'The "target" argument must be of type ' + "Buffer or Uint8Array." + common.invalidArgTypeHelper('abc') }); diff --git a/test/js/node/test/parallel/test-buffer-concat.js b/test/js/node/test/parallel/test-buffer-concat.js index 2e7541fb58..93667f54d2 100644 --- a/test/js/node/test/parallel/test-buffer-concat.js +++ b/test/js/node/test/parallel/test-buffer-concat.js @@ -49,7 +49,7 @@ assert.strictEqual(flatLongLen.toString(), check); Buffer.concat(value); }, { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "list" argument must be an instance of Array.' + + message: 'The "list" argument must be of type Array.' + `${common.invalidArgTypeHelper(value)}` }); }); @@ -59,7 +59,7 @@ assert.strictEqual(flatLongLen.toString(), check); Buffer.concat(value); }, { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "list[0]" argument must be an instance of Buffer ' + + message: 'The "list[0]" argument must be of type Buffer ' + `or Uint8Array.${common.invalidArgTypeHelper(value[0])}` }); }); @@ -68,7 +68,7 @@ assert.throws(() => { Buffer.concat([Buffer.from('hello'), 3]); }, { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "list[1]" argument must be an instance of Buffer ' + + message: 'The "list[1]" argument must be of type Buffer ' + 'or Uint8Array. Received type number (3)' }); diff --git a/test/js/node/test/parallel/test-buffer-equals.js b/test/js/node/test/parallel/test-buffer-equals.js index 18f0e17836..f685d01499 100644 --- a/test/js/node/test/parallel/test-buffer-equals.js +++ b/test/js/node/test/parallel/test-buffer-equals.js @@ -19,7 +19,7 @@ assert.throws( { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "otherBuffer" argument must be an instance of ' + + message: 'The "otherBuffer" argument must be of type ' + "Buffer or Uint8Array." + common.invalidArgTypeHelper('abc') } ); diff --git a/test/js/node/test/parallel/test-dns-setservers-type-check.js b/test/js/node/test/parallel/test-dns-setservers-type-check.js index 7a19dc5eb0..16bfe90af4 100644 --- a/test/js/node/test/parallel/test-dns-setservers-type-check.js +++ b/test/js/node/test/parallel/test-dns-setservers-type-check.js @@ -20,7 +20,7 @@ const promiseResolver = new dns.promises.Resolver(); const errObj = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: /^The "servers" argument must be an instance of Array\./ + message: /^The "servers" argument must be of type Array\./ }; assert.throws( () => { diff --git a/test/js/node/test/parallel/test-process-hrtime.js b/test/js/node/test/parallel/test-process-hrtime.js index 34ef514aac..7295404997 100644 --- a/test/js/node/test/parallel/test-process-hrtime.js +++ b/test/js/node/test/parallel/test-process-hrtime.js @@ -38,7 +38,7 @@ assert.throws(() => { }, { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "time" argument must be an instance of Array. Received type ' + + message: 'The "time" argument must be of type Array. Received type ' + 'number (1)' }); assert.throws(() => { diff --git a/test/js/node/test/parallel/test-process-setgroups.js b/test/js/node/test/parallel/test-process-setgroups.js index c26b5dbaf1..99d2ed834f 100644 --- a/test/js/node/test/parallel/test-process-setgroups.js +++ b/test/js/node/test/parallel/test-process-setgroups.js @@ -17,7 +17,7 @@ assert.throws( { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "groups" argument must be an instance of Array. ' + + message: 'The "groups" argument must be of type Array. ' + 'Received undefined' } ); diff --git a/test/js/node/test/parallel/test-string-decoder.js b/test/js/node/test/parallel/test-string-decoder.js index d82a149bf2..9889e212ff 100644 --- a/test/js/node/test/parallel/test-string-decoder.js +++ b/test/js/node/test/parallel/test-string-decoder.js @@ -196,7 +196,7 @@ assert.throws( { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "buf" argument must be an instance of Buffer, TypedArray,' + + message: 'The "buf" argument must be of type Buffer, TypedArray,' + ' or DataView. Received null' } );