From 4d6480050ce6629074fc1db2c1c1be45a19a9bf2 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Mon, 29 Jul 2024 19:15:23 -0700 Subject: [PATCH] NodeError: add more and use them in child_process and dgram (#12929) --- src/bun.js/bindings/NodeError.cpp | 120 +++++++++++++++++- src/bun.js/bindings/NodeError.h | 4 + src/bun.js/bindings/bindings.zig | 15 +++ src/js/internal/errors.ts | 4 + src/js/node/child_process.ts | 21 +-- src/js/node/dgram.ts | 1 + src/js/node/http.ts | 43 +------ .../child_process/child_process-node.test.js | 16 +-- 8 files changed, 156 insertions(+), 68 deletions(-) diff --git a/src/bun.js/bindings/NodeError.cpp b/src/bun.js/bindings/NodeError.cpp index c29dd34338..66e46f8ab1 100644 --- a/src/bun.js/bindings/NodeError.cpp +++ b/src/bun.js/bindings/NodeError.cpp @@ -5,17 +5,60 @@ #include "JavaScriptCore/JSCJSValue.h" #include "JavaScriptCore/ErrorInstance.h" #include "JavaScriptCore/ExceptionScope.h" +#include "JavaScriptCore/JSString.h" +#include "JavaScriptCore/JSType.h" +#include "JavaScriptCore/Symbol.h" #include "wtf/text/ASCIILiteral.h" #include "wtf/text/MakeString.h" +#include "wtf/text/WTFString.h" #include JSC::EncodedJSValue JSC__JSValue__createTypeError(const ZigString* message, const ZigString* arg1, JSC::JSGlobalObject* globalObject); JSC::EncodedJSValue JSC__JSValue__createRangeError(const ZigString* message, const ZigString* arg1, JSC::JSGlobalObject* globalObject); +extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue val_arg_name, JSC::EncodedJSValue val_expected_type, JSC::EncodedJSValue val_actual_value); +extern "C" JSC::EncodedJSValue Bun__ERR_MISSING_ARGS(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue arg1, JSC::EncodedJSValue arg2, JSC::EncodedJSValue arg3); +extern "C" JSC::EncodedJSValue Bun__ERR_IPC_CHANNEL_CLOSED(JSC::JSGlobalObject* globalObject); + namespace Bun { using namespace JSC; +WTF::String JSValueToStringSafe(JSC::JSGlobalObject* globalObject, JSValue arg) +{ + ASSERT(!arg.isEmpty()); + if (!arg.isCell()) + return arg.toString(globalObject)->getString(globalObject); + + auto cell = arg.asCell(); + auto jstype = cell->type(); + + if (jstype == JSC::JSType::StringType) { + return cell->toStringInline(globalObject)->getString(globalObject); + } + if (jstype == JSC::JSType::SymbolType) { + auto symbol = jsCast(cell); + auto result = symbol->tryGetDescriptiveString(); + if (result.has_value()) + return result.value(); + } + return arg.toString(globalObject)->getString(globalObject); +} + +JSC::JSValue createErrorWithCode(JSC::JSGlobalObject* globalObject, String message, ASCIILiteral code) +{ + JSC::VM& vm = globalObject->vm(); + + JSC::JSObject* result = JSC::createError(globalObject, message); + JSC::EnsureStillAliveScope ensureAlive(result); + auto typeError = JSC::JSValue(result).asCell()->getObject(); + + auto clientData = WebCore::clientData(vm); + typeError->putDirect(vm, clientData->builtinNames().codePublicName(), jsString(vm, String(code)), 0); + + return typeError; +} + JSC::JSValue createTypeErrorWithCode(JSC::JSGlobalObject* globalObject, String message, ASCIILiteral code) { JSC::VM& vm = globalObject->vm(); @@ -54,17 +97,26 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_INVALID_ARG_TYPE, (JSC::JSGlobalObject * JSC::throwTypeError(globalObject, scope, "requires 3 arguments"_s); return {}; } + auto arg_name = callFrame->argument(0); + auto expected_type = callFrame->argument(1); + auto actual_value = callFrame->argument(2); + return Bun__ERR_INVALID_ARG_TYPE(globalObject, JSValue::encode(arg_name), JSValue::encode(expected_type), JSValue::encode(actual_value)); +} +extern "C" JSC::EncodedJSValue Bun__ERR_INVALID_ARG_TYPE(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue val_arg_name, JSC::EncodedJSValue val_expected_type, JSC::EncodedJSValue val_actual_value) +{ + JSC::VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); - auto arg_name = callFrame->argument(0).toWTFString(globalObject); + auto arg_name = JSValue::decode(val_arg_name).toWTFString(globalObject); RETURN_IF_EXCEPTION(scope, {}); - auto expected_type = callFrame->argument(1).toWTFString(globalObject); + auto expected_type = JSValue::decode(val_expected_type).toWTFString(globalObject); RETURN_IF_EXCEPTION(scope, {}); - auto actual_value = callFrame->argument(2).toWTFString(globalObject); + auto actual_value = JSValueToStringSafe(globalObject, JSValue::decode(val_actual_value)); RETURN_IF_EXCEPTION(scope, {}); - auto message = makeString("The \""_s, arg_name, "\" argument must be of type "_s, expected_type, ". Recieved "_s, actual_value); + auto message = makeString("The \""_s, arg_name, "\" argument must be of type "_s, expected_type, ". Received "_s, actual_value); return JSC::JSValue::encode(createTypeErrorWithCode(globalObject, message, "ERR_INVALID_ARG_TYPE"_s)); } @@ -92,4 +144,64 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_OUT_OF_RANGE, (JSC::JSGlobalObject * glo return JSC::JSValue::encode(createRangeErrorWithCode(globalObject, message, "ERR_OUT_OF_RANGE"_s)); } +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_IPC_DISCONNECTED, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)) +{ + return JSC::JSValue::encode(createErrorWithCode(globalObject, "IPC channel is already disconnected"_s, "ERR_IPC_DISCONNECTED"_s)); +} + +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_SERVER_NOT_RUNNING, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)) +{ + return JSC::JSValue::encode(createErrorWithCode(globalObject, "Server is not running."_s, "ERR_SERVER_NOT_RUNNING"_s)); +} + +extern "C" JSC::EncodedJSValue Bun__ERR_MISSING_ARGS(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue arg1, JSC::EncodedJSValue arg2, JSC::EncodedJSValue arg3) +{ + JSC::VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + + if (arg1 == 0) { + JSC::throwTypeError(globalObject, scope, "requires at least 1 argument"_s); + return {}; + } + + auto name1 = JSValue::decode(arg1).toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + + if (arg2 == 0) { + // 1 arg name passed + auto message = makeString("The \""_s, name1, "\" argument must be specified"_s); + return JSC::JSValue::encode(createTypeErrorWithCode(globalObject, message, "ERR_MISSING_ARGS"_s)); + } + + auto name2 = JSValue::decode(arg2).toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + + if (arg3 == 0) { + // 2 arg names passed + auto message = makeString("The \""_s, name1, "\" and \""_s, name2, "\" arguments must be specified"_s); + return JSC::JSValue::encode(createTypeErrorWithCode(globalObject, message, "ERR_MISSING_ARGS"_s)); + } + + auto name3 = JSValue::decode(arg3).toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + + // 3 arg names passed + auto message = makeString("The \""_s, name1, "\", \""_s, name2, "\", and \""_s, name3, "\" arguments must be specified"_s); + return JSC::JSValue::encode(createTypeErrorWithCode(globalObject, message, "ERR_MISSING_ARGS"_s)); +} + +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_IPC_CHANNEL_CLOSED, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)) +{ + return Bun__ERR_IPC_CHANNEL_CLOSED(globalObject); +} +extern "C" JSC::EncodedJSValue Bun__ERR_IPC_CHANNEL_CLOSED(JSC::JSGlobalObject* globalObject) +{ + return JSC::JSValue::encode(createErrorWithCode(globalObject, "Channel closed."_s, "ERR_IPC_CHANNEL_CLOSED"_s)); +} + +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_SOCKET_BAD_TYPE, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)) +{ + return JSC::JSValue::encode(createTypeErrorWithCode(globalObject, "Bad socket type specified. Valid types are: udp4, udp6"_s, "ERR_SOCKET_BAD_TYPE"_s)); +} + } diff --git a/src/bun.js/bindings/NodeError.h b/src/bun.js/bindings/NodeError.h index dc959feff0..d1e72be077 100644 --- a/src/bun.js/bindings/NodeError.h +++ b/src/bun.js/bindings/NodeError.h @@ -4,5 +4,9 @@ namespace Bun { JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_INVALID_ARG_TYPE, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)); JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_OUT_OF_RANGE, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)); +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_IPC_DISCONNECTED, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)); +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_SERVER_NOT_RUNNING, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)); +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_IPC_CHANNEL_CLOSED, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)); +JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_SOCKET_BAD_TYPE, (JSC::JSGlobalObject * globalObject, JSC::CallFrame*)); } diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index d1aa20bf0e..5bba774ed9 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -97,6 +97,21 @@ pub const JSObject = extern struct { }); } + extern fn Bun__ERR_INVALID_ARG_TYPE(*JSGlobalObject, JSValue, JSValue, JSValue) JSValue; + pub fn ERR_INVALID_ARG_TYPE(this: *JSGlobalObject, arg_name: JSValue, etype: JSValue, atype: JSValue) JSValue { + return Bun__ERR_INVALID_ARG_TYPE(this, arg_name, etype, atype); + } + + extern fn Bun__ERR_MISSING_ARGS(*JSGlobalObject, JSValue, JSValue, JSValue) JSValue; + pub fn ERR_MISSING_ARGS(this: *JSGlobalObject, arg1: JSValue, arg2: JSValue, arg3: JSValue) JSValue { + return Bun__ERR_MISSING_ARGS(this, arg1, arg2, arg3); + } + + extern fn Bun__ERR_IPC_CHANNEL_CLOSED(*JSGlobalObject) JSValue; + pub fn ERR_IPC_CHANNEL_CLOSED(this: *JSGlobalObject) JSValue { + return Bun__ERR_IPC_CHANNEL_CLOSED(this); + } + pub const Extern = [_][]const u8{ "putRecord", "create", diff --git a/src/js/internal/errors.ts b/src/js/internal/errors.ts index 034f33b457..e6f6051235 100644 --- a/src/js/internal/errors.ts +++ b/src/js/internal/errors.ts @@ -1,4 +1,8 @@ export default { ERR_INVALID_ARG_TYPE: $newCppFunction("NodeError.cpp", "jsFunction_ERR_INVALID_ARG_TYPE", 3), ERR_OUT_OF_RANGE: $newCppFunction("NodeError.cpp", "jsFunction_ERR_OUT_OF_RANGE", 3), + ERR_IPC_DISCONNECTED: $newCppFunction("NodeError.cpp", "jsFunction_ERR_IPC_DISCONNECTED", 0), + ERR_SERVER_NOT_RUNNING: $newCppFunction("NodeError.cpp", "jsFunction_ERR_SERVER_NOT_RUNNING", 0), + ERR_IPC_CHANNEL_CLOSED: $newCppFunction("NodeError.cpp", "jsFunction_ERR_IPC_CHANNEL_CLOSED", 0), + ERR_SOCKET_BAD_TYPE: $newCppFunction("NodeError.cpp", "jsFunction_ERR_SOCKET_BAD_TYPE", 0), }; diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index fe48fcefae..2e6e427ff2 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -2,12 +2,13 @@ const EventEmitter = require("node:events"); const StreamModule = require("node:stream"); const OsModule = require("node:os"); +const { ERR_INVALID_ARG_TYPE, ERR_IPC_DISCONNECTED } = require("internal/errors"); +const { kHandle } = require("internal/shared"); var NetModule; var ObjectCreate = Object.create; var ObjectAssign = Object.assign; -var ObjectDefineProperty = Object.defineProperty; var BufferConcat = Buffer.concat; var BufferIsEncoding = Buffer.isEncoding; @@ -21,7 +22,6 @@ var ArrayPrototypeIncludes = Array.prototype.includes; var ArrayPrototypeSlice = Array.prototype.slice; var ArrayPrototypeUnshift = Array.prototype.unshift; -// var ArrayBuffer = ArrayBuffer; var ArrayBufferIsView = ArrayBuffer.isView; var NumberIsInteger = Number.isInteger; @@ -104,17 +104,6 @@ var ReadableFromWeb; // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -function spawnTimeoutFunction(child, timeoutHolder) { - var timeoutId = timeoutHolder.timeoutId; - if (timeoutId > -1) { - try { - child.kill(killSignal); - } catch (err) { - child.emit("error", err); - } - timeoutHolder.timeoutId = -1; - } -} /** * Spawns a new process using the given `file`. * @param {string} file @@ -1943,12 +1932,6 @@ function ERR_UNKNOWN_SIGNAL(name) { return err; } -function ERR_INVALID_ARG_TYPE(name, type, value) { - const err = new TypeError(`The "${name}" argument must be of type ${type}. Received ${value?.toString()}`); - err.code = "ERR_INVALID_ARG_TYPE"; - return err; -} - function ERR_INVALID_OPT_VALUE(name, value) { const err = new TypeError(`The value "${value}" is invalid for option "${name}"`); err.code = "ERR_INVALID_OPT_VALUE"; diff --git a/src/js/node/dgram.ts b/src/js/node/dgram.ts index 8762c82b0a..c549ffb4cf 100644 --- a/src/js/node/dgram.ts +++ b/src/js/node/dgram.ts @@ -34,6 +34,7 @@ const kStateSymbol = Symbol("state symbol"); const async_id_symbol = Symbol("async_id_symbol"); const { hideFromStack, throwNotImplemented } = require("internal/shared"); +const { ERR_SOCKET_BAD_TYPE } = require("internal/errors"); const { FunctionPrototypeBind, diff --git a/src/js/node/http.ts b/src/js/node/http.ts index 45c8efefb8..4ff4aa4d96 100644 --- a/src/js/node/http.ts +++ b/src/js/node/http.ts @@ -1,7 +1,8 @@ // Hardcoded module "node:http" const EventEmitter = require("node:events"); const { isTypedArray } = require("node:util/types"); -const { Duplex, Readable, Writable, ERR_STREAM_WRITE_AFTER_END, ERR_STREAM_ALREADY_FINISHED } = require("node:stream"); +const { Duplex, Readable, Writable } = require("node:stream"); +const { ERR_INVALID_ARG_TYPE } = require("internal/errors"); const { getHeader, @@ -64,27 +65,6 @@ function ERR_HTTP_SOCKET_ASSIGNED() { return new Error(`ServerResponse has an already assigned socket`); } -// Cheaper to duplicate this than to import it from node:net -function isIPv6(input) { - const v4Seg = "(?:[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])"; - const v4Str = `(${v4Seg}[.]){3}${v4Seg}`; - const v6Seg = "(?:[0-9a-fA-F]{1,4})"; - const IPv6Reg = new RegExp( - "^(" + - `(?:${v6Seg}:){7}(?:${v6Seg}|:)|` + - `(?:${v6Seg}:){6}(?:${v4Str}|:${v6Seg}|:)|` + - `(?:${v6Seg}:){5}(?::${v4Str}|(:${v6Seg}){1,2}|:)|` + - `(?:${v6Seg}:){4}(?:(:${v6Seg}){0,1}:${v4Str}|(:${v6Seg}){1,3}|:)|` + - `(?:${v6Seg}:){3}(?:(:${v6Seg}){0,2}:${v4Str}|(:${v6Seg}){1,4}|:)|` + - `(?:${v6Seg}:){2}(?:(:${v6Seg}){0,3}:${v4Str}|(:${v6Seg}){1,5}|:)|` + - `(?:${v6Seg}:){1}(?:(:${v6Seg}){0,4}:${v4Str}|(:${v6Seg}){1,6}|:)|` + - `(?::((?::${v6Seg}){0,5}:${v4Str}|(?::${v6Seg}){1,7}|:))` + - ")(%[0-9a-zA-Z-.:]{1,})?$", - ); - - return IPv6Reg.test(input); -} - // TODO: add primordial for URL // Importing from node:url is unnecessary const { URL } = globalThis; @@ -95,14 +75,10 @@ const fetch = Bun.fetch; const nop = () => {}; const kEmptyObject = Object.freeze(Object.create(null)); -const kOutHeaders = Symbol.for("kOutHeaders"); const kEndCalled = Symbol.for("kEndCalled"); const kAbortController = Symbol.for("kAbortController"); const kClearTimeout = Symbol("kClearTimeout"); -const kCorked = Symbol.for("kCorked"); -const searchParamsSymbol = Symbol.for("query"); // This is the symbol used in Node - // Primordials const StringPrototypeSlice = String.prototype.slice; const StringPrototypeStartsWith = String.prototype.startsWith; @@ -134,23 +110,16 @@ function isValidTLSArray(obj) { return false; } -class ERR_INVALID_ARG_TYPE extends TypeError { - constructor(name, expected, actual) { - super(`The ${name} argument must be of type ${expected}. Received type ${typeof actual}`); - this.code = "ERR_INVALID_ARG_TYPE"; - } -} - function validateMsecs(numberlike: any, field: string) { if (typeof numberlike !== "number" || numberlike < 0) { - throw new ERR_INVALID_ARG_TYPE(field, "number", numberlike); + throw ERR_INVALID_ARG_TYPE(field, "number", numberlike); } return numberlike; } function validateFunction(callable: any, field: string) { if (typeof callable !== "function") { - throw new ERR_INVALID_ARG_TYPE(field, "Function", callable); + throw ERR_INVALID_ARG_TYPE(field, "Function", callable); } return callable; @@ -1676,7 +1645,7 @@ class ClientRequest extends OutgoingMessage { let method = options.method; const methodIsString = typeof method === "string"; if (method !== null && method !== undefined && !methodIsString) { - // throw new ERR_INVALID_ARG_TYPE("options.method", "string", method); + // throw ERR_INVALID_ARG_TYPE("options.method", "string", method); throw new Error("ERR_INVALID_ARG_TYPE: options.method"); } @@ -1925,7 +1894,7 @@ function urlToHttpOptions(url) { function validateHost(host, name) { if (host !== null && host !== undefined && typeof host !== "string") { - // throw new ERR_INVALID_ARG_TYPE( + // throw ERR_INVALID_ARG_TYPE( // `options.${name}`, // ["string", "undefined", "null"], // host, diff --git a/test/js/node/child_process/child_process-node.test.js b/test/js/node/child_process/child_process-node.test.js index 7bd4be6f09..72e930f376 100644 --- a/test/js/node/child_process/child_process-node.test.js +++ b/test/js/node/child_process/child_process-node.test.js @@ -651,13 +651,13 @@ describe("fork", () => { }); }); describe("args", () => { - it("Ensure that first argument `modulePath` must be provided and be of type string", () => { - const invalidModulePath = [0, true, undefined, null, [], {}, () => {}, Symbol("t")]; - invalidModulePath.forEach(modulePath => { + const invalidModulePath = [0, true, undefined, null, [], {}, () => {}, Symbol("t")]; + invalidModulePath.forEach(modulePath => { + it(`Ensure that first argument \`modulePath\` must be provided and be of type string :: ${String(modulePath)}`, () => { expect(() => fork(modulePath, { env: bunEnv })).toThrow({ code: "ERR_INVALID_ARG_TYPE", name: "TypeError", - message: `The "modulePath" argument must be of type string,Buffer,URL. Received ${modulePath?.toString()}`, + message: `The "modulePath" argument must be of type string,Buffer,URL. Received ${String(modulePath)}`, }); }); }); @@ -705,15 +705,15 @@ describe("fork", () => { }); }, ); - it("Ensure that the third argument should be type of object if provided", () => { - const invalidThirdArgs = [0, true, () => {}, Symbol("t")]; - invalidThirdArgs.forEach(arg => { + const invalidThirdArgs = [0, true, () => {}, Symbol("t")]; + invalidThirdArgs.forEach(arg => { + it(`Ensure that the third argument should be type of object if provided :: ${String(arg)}`, () => { expect(() => { fork(fixtures.path("child-process-echo-options.js"), [], arg); }).toThrow({ code: "ERR_INVALID_ARG_TYPE", name: "TypeError", - message: `The \"options\" argument must be of type object. Received ${arg?.toString()}`, + message: `The "options" argument must be of type object. Received ${String(arg)}`, }); }); });