From 37d71cb9a423537c110525a89499cf4949a9aada Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Tue, 7 May 2024 15:03:45 -0700 Subject: [PATCH 01/10] prototype of uncaughtException handler and friends --- src/bun.js/api/BunObject.zig | 8 ++--- src/bun.js/api/bun/socket.zig | 4 +-- src/bun.js/api/bun/udp_socket.zig | 4 +-- src/bun.js/api/html_rewriter.zig | 2 +- src/bun.js/api/server.zig | 6 ++-- src/bun.js/bindings/BunProcess.cpp | 41 ++++++++++++++++++++++++ src/bun.js/bindings/bindings.zig | 10 ++++++ src/bun.js/event_loop.zig | 2 +- src/bun.js/javascript.zig | 33 +++++++++++++------ src/bun.js/node/node_fs_stat_watcher.zig | 4 +-- src/bun.js/node/node_fs_watcher.zig | 2 +- src/bun.js/test/jest.zig | 24 +++++++------- src/bun.js/web_worker.zig | 3 +- src/bun_js.zig | 11 ++++--- src/cli/test_command.zig | 4 +-- src/js_ast.zig | 22 ++++++------- src/napi/napi.zig | 2 +- 17 files changed, 122 insertions(+), 60 deletions(-) diff --git a/src/bun.js/api/BunObject.zig b/src/bun.js/api/BunObject.zig index d2bfc5d3cd038a..d124278c2d7e2d 100644 --- a/src/bun.js/api/BunObject.zig +++ b/src/bun.js/api/BunObject.zig @@ -3685,7 +3685,7 @@ pub const Timer = struct { } var this = args.ptr[1].asPtr(CallbackJob); - globalThis.bunVM().onError(globalThis, args.ptr[0]); + _ = globalThis.bunVM().uncaughtException(globalThis, args.ptr[0], .null); this.deinit(); return JSValue.jsUndefined(); } @@ -3787,7 +3787,7 @@ pub const Timer = struct { } if (result.isAnyError()) { - vm.onError(globalThis, result); + _ = vm.uncaughtException(globalThis, result, JSC.JSValue.null); this.deinit(); return; } @@ -3796,7 +3796,7 @@ pub const Timer = struct { switch (promise.status(globalThis.vm())) { .Rejected => { this.deinit(); - vm.onError(globalThis, promise.result(globalThis.vm())); + _ = vm.unhandledRejection(globalThis, promise.result(globalThis.vm()), promise.asValue(globalThis)); }, .Fulfilled => { this.deinit(); @@ -5265,7 +5265,7 @@ pub const EnvironmentVariables = struct { }; export fn Bun__reportError(globalObject: *JSGlobalObject, err: JSC.JSValue) void { - JSC.VirtualMachine.get().onError(globalObject, err); + _ = JSC.VirtualMachine.get().uncaughtException(globalObject, err, .null); } comptime { diff --git a/src/bun.js/api/bun/socket.zig b/src/bun.js/api/bun/socket.zig index 1b659b6692f6ad..86ca8fc2661b6a 100644 --- a/src/bun.js/api/bun/socket.zig +++ b/src/bun.js/api/bun/socket.zig @@ -236,14 +236,14 @@ const Handlers = struct { const onError = this.onError; if (onError == .zero) { if (err.len > 0) - this.vm.onError(this.globalObject, err[0]); + _ = this.vm.uncaughtException(this.globalObject, err[0], JSC.JSValue.null); return false; } const result = onError.callWithThis(this.globalObject, thisValue, err); if (result.isAnyError()) { - this.vm.onError(this.globalObject, result); + _ = this.vm.uncaughtException(this.globalObject, result, JSC.JSValue.null); } return true; diff --git a/src/bun.js/api/bun/udp_socket.zig b/src/bun.js/api/bun/udp_socket.zig index 799b4b7bfc2bc8..5418d775df2272 100644 --- a/src/bun.js/api/bun/udp_socket.zig +++ b/src/bun.js/api/bun/udp_socket.zig @@ -359,14 +359,14 @@ pub const UDPSocket = struct { if (callback == .zero) { if (err.len > 0) - vm.onError(globalThis, err[0]); + _ = vm.uncaughtException(globalThis, err[0], JSC.JSValue.null); return false; } const result = callback.callWithThis(globalThis, thisValue, err); if (result.isAnyError()) { - vm.onError(globalThis, result); + _ = vm.uncaughtException(globalThis, result, JSC.JSValue.null); } return true; diff --git a/src/bun.js/api/html_rewriter.zig b/src/bun.js/api/html_rewriter.zig index 799af4d6a0bdad..04d86bf989cefd 100644 --- a/src/bun.js/api/html_rewriter.zig +++ b/src/bun.js/api/html_rewriter.zig @@ -897,7 +897,7 @@ fn HandlerCallback( this.global.bunVM().waitForPromise(promise); const fail = promise.status(this.global.vm()) == .Rejected; if (fail) { - this.global.bunVM().onError(this.global, promise.result(this.global.vm())); + _ = this.global.bunVM().unhandledRejection(this.global, promise.result(this.global.vm()), promise.asValue(this.global)); } return fail; } diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index a55d3b8b15be92..5709ddb8d64e4e 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -3930,7 +3930,7 @@ pub const ServerWebSocket = struct { } if (error_handler.isEmptyOrUndefinedOrNull()) { - vm.onError(globalObject, err_value); + _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); } else { const corky = [_]JSValue{err_value}; corker.args = &corky; @@ -3997,7 +3997,7 @@ pub const ServerWebSocket = struct { if (result.toError()) |err_value| { if (this.handler.onError.isEmptyOrUndefinedOrNull()) { - vm.onError(globalObject, err_value); + _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); } else { const args = [_]JSValue{err_value}; corker.args = &args; @@ -4049,7 +4049,7 @@ pub const ServerWebSocket = struct { if (result.toError()) |err_value| { if (this.handler.onError.isEmptyOrUndefinedOrNull()) { - vm.onError(globalObject, err_value); + _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); } else { const args = [_]JSValue{err_value}; corker.args = &args; diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index fa4d8aecc3ec71..76c457641eae40 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -3,6 +3,7 @@ #include #include #include +#include "JavaScriptCore/JSCJSValue.h" #include "ScriptExecutionContext.h" #include "headers-handwritten.h" #include "node_api.h" @@ -746,6 +747,40 @@ void signalHandler(uv_signal_t* signal, int signalNumber) }); }; +extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue exception, JSC::JSValue origin) +{ + auto* globalObject = jsCast(lexicalGlobalObject); + auto* process = jsCast(globalObject->processObject()); + MarkedArgumentBuffer args; + args.append(exception); + args.append(origin); + auto eventType = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); + auto& wrapped = process->wrapped(); + if (wrapped.listenerCount(eventType) > 0) { + wrapped.emit(eventType, args); + return true; + } else { + return false; + } +} + +extern "C" int Bun__handleUnhandledRejection(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue reason, JSC::JSValue promise) +{ + auto* globalObject = jsCast(lexicalGlobalObject); + auto* process = jsCast(globalObject->processObject()); + MarkedArgumentBuffer args; + args.append(reason); + args.append(promise); + auto eventType = Identifier::fromString(globalObject->vm(), "unhandledRejection"_s); + auto& wrapped = process->wrapped(); + if (wrapped.listenerCount(eventType) > 0) { + wrapped.emit(eventType, args); + return true; + } else { + return false; + } +} + static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& eventName, bool isAdded) { if (eventEmitter.scriptExecutionContext()->isMainThread()) { @@ -767,6 +802,12 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e return; } + if (eventName.string() == "uncaughtException"_s) { + if (isAdded) { + + } + } + // Signal Handlers loadSignalNumberMap(); static std::once_flag signalNumberToNameMapOnceFlag; diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index a8df3757d4ca29..01a23a31bca99e 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -2487,6 +2487,10 @@ pub const JSInternalPromise = extern struct { return cppFn("create", .{globalThis}); } + pub fn asValue(this: *JSInternalPromise) JSValue { + return JSValue.fromCell(this); + } + pub const Extern = [_][]const u8{ "create", // "then_", @@ -2558,6 +2562,12 @@ pub const AnyPromise = union(enum) { inline else => |promise| promise.rejectAsHandledException(globalThis, value), } } + pub fn asValue(this: AnyPromise, globalThis: *JSGlobalObject) JSValue { + return switch (this) { + .Normal => |promise| promise.asValue(globalThis), + .Internal => |promise| promise.asValue(), + }; + } }; // SourceProvider.h diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index dc12ab3266beaa..4499058a896249 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -855,7 +855,7 @@ pub const EventLoop = struct { const result = callback.callWithThis(globalObject, thisValue, arguments); if (result.toError()) |err| { - this.virtual_machine.onError(globalObject, err); + _ = this.virtual_machine.uncaughtException(globalObject, err, .null); } } diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 8ab9cc78baefcf..a31f875d2a9d78 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -351,7 +351,7 @@ pub export fn Bun__reportUnhandledError(globalObject: *JSGlobalObject, value: JS // This JSGlobalObject might not be the main script execution context // See the crash in https://github.com/oven-sh/bun/issues/9778 const jsc_vm = JSC.VirtualMachine.get(); - jsc_vm.onError(globalObject, value); + _ = jsc_vm.uncaughtException(globalObject, value, .undefined); return JSC.JSValue.jsUndefined(); } @@ -379,7 +379,7 @@ pub export fn Bun__handleRejectedPromise(global: *JSGlobalObject, promise: *JSC. if (result == .zero) return; - jsc_vm.onError(global, result); + _ = jsc_vm.unhandledRejection(global, result, promise.asValue(global)); jsc_vm.autoGarbageCollect(); } @@ -774,10 +774,6 @@ pub const VirtualMachine = struct { }; } - pub fn resetUnhandledRejection(this: *VirtualMachine) void { - this.onUnhandledRejection = defaultOnUnhandledRejection; - } - pub fn loadExtraEnv(this: *VirtualMachine) void { var map = this.bundler.env.map; @@ -825,9 +821,26 @@ pub const VirtualMachine = struct { } } - pub fn onError(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, value: JSC.JSValue) void { - this.unhandled_error_counter += 1; - this.onUnhandledRejection(this, globalObject, value); + extern fn Bun__handleUncaughtException(*JSC.JSGlobalObject, err: JSC.JSValue, origin: JSC.JSValue) c_int; + extern fn Bun__handleUnhandledRejection(*JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) c_int; + + pub fn unhandledRejection(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) bool { + const handled = Bun__handleUnhandledRejection(globalObject, reason, promise) > 0; + if (!handled) { + this.unhandled_error_counter += 1; + this.onUnhandledRejection(this, globalObject, reason); + } + return handled; + } + + pub fn uncaughtException(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, err: JSC.JSValue, origin: JSC.JSValue) bool { + const handled = Bun__handleUncaughtException(globalObject, err, origin) > 0; + if (!handled) { + // TODO maybe we want a separate code path for uncaught exceptions + this.unhandled_error_counter += 1; + this.onUnhandledRejection(this, globalObject, err); + } + return handled; } pub fn defaultOnUnhandledRejection(this: *JSC.VirtualMachine, _: *JSC.JSGlobalObject, value: JSC.JSValue) void { @@ -2614,7 +2627,7 @@ pub const VirtualMachine = struct { pub fn reportUncaughtException(globalObject: *JSGlobalObject, exception: *JSC.Exception) JSValue { var jsc_vm = globalObject.bunVM(); - jsc_vm.onError(globalObject, exception.value()); + _ = jsc_vm.uncaughtException(globalObject, exception.value(), JSC.JSValue.jsUndefined()); return JSC.JSValue.jsUndefined(); } diff --git a/src/bun.js/node/node_fs_stat_watcher.zig b/src/bun.js/node/node_fs_stat_watcher.zig index fe5e3d5dfff6a1..e0bbd78fdb7264 100644 --- a/src/bun.js/node/node_fs_stat_watcher.zig +++ b/src/bun.js/node/node_fs_stat_watcher.zig @@ -385,7 +385,7 @@ pub const StatWatcher = struct { const vm = this.globalThis.bunVM(); if (result.isAnyError()) { - vm.onError(this.globalThis, result); + _ = vm.uncaughtException(this.globalThis, result, JSC.JSValue.null); } vm.rareData().nodeFSStatWatcherScheduler(vm).append(this); @@ -421,7 +421,7 @@ pub const StatWatcher = struct { ); if (result.isAnyError()) { const vm = this.globalThis.bunVM(); - vm.onError(this.globalThis, result); + _ = vm.uncaughtException(this.globalThis, result, JSC.JSValue.null); } } diff --git a/src/bun.js/node/node_fs_watcher.zig b/src/bun.js/node/node_fs_watcher.zig index 09a3707b1ff3f2..d4061c43c7e5c0 100644 --- a/src/bun.js/node/node_fs_watcher.zig +++ b/src/bun.js/node/node_fs_watcher.zig @@ -606,7 +606,7 @@ pub const FSWatcher = struct { ); if (err.toError()) |value| { - JSC.VirtualMachine.get().onError(globalObject, value); + _ = JSC.VirtualMachine.get().uncaughtException(globalObject, value, JSC.JSValue.null); } } diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig index f95a79dd8d92a5..73160a957618b3 100644 --- a/src/bun.js/test/jest.zig +++ b/src/bun.js/test/jest.zig @@ -602,7 +602,7 @@ pub const TestScope = struct { debug("onReject", .{}); const arguments = callframe.arguments(2); const err = arguments.ptr[0]; - globalThis.bunVM().onError(globalThis, err); + _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); var task: *TestRunnerTask = arguments.ptr[1].asPromisePtr(TestRunnerTask); task.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .promise); globalThis.bunVM().autoGarbageCollect(); @@ -637,7 +637,7 @@ pub const TestScope = struct { task.handleResult(.{ .pass = expect.active_test_expectation_counter.actual }, .callback); } else { debug("done(err)", .{}); - globalThis.bunVM().onError(globalThis, err); + _ = globalThis.bunVM().uncaughtException(globalThis, err, JSC.JSValue.null); task.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .callback); } } else { @@ -696,7 +696,7 @@ pub const TestScope = struct { initial_value = callJSFunctionForTestRunner(vm, vm.global, this.func, this.func_arg); if (initial_value.isAnyError()) { - vm.onError(vm.global, initial_value); + _ = vm.uncaughtException(vm.global, initial_value, JSC.JSValue.null); if (this.tag == .todo) { return .{ .todo = {} }; @@ -719,7 +719,7 @@ pub const TestScope = struct { } switch (promise.status(vm.global.vm())) { .Rejected => { - vm.onError(vm.global, promise.result(vm.global.vm())); + _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue(vm.global)); if (this.tag == .todo) { return .{ .todo = {} }; @@ -893,7 +893,7 @@ pub const DescribeScope = struct { if (args.len > 0) { const err = args.ptr[0]; if (!err.isEmptyOrUndefinedOrNull()) { - ctx.bunVM().onError(ctx.bunVM().global, err); + _ = ctx.bunVM().uncaughtException(ctx.bunVM().global, err, JSC.JSValue.null); } } scope.done = true; @@ -1090,12 +1090,12 @@ pub const DescribeScope = struct { switch (prom.status(globalObject.ptr().vm())) { JSPromise.Status.Fulfilled => {}, else => { - globalObject.bunVM().onError(globalObject, prom.result(globalObject.ptr().vm())); + _ = globalObject.bunVM().unhandledRejection(globalObject, prom.result(globalObject.ptr().vm()), prom.asValue(globalObject)); return .undefined; }, } } else if (result.toError()) |err| { - globalObject.bunVM().onError(globalObject, err); + _ = globalObject.bunVM().uncaughtException(globalObject, err, .null); return .undefined; } } @@ -1123,7 +1123,7 @@ pub const DescribeScope = struct { if (this.shouldEvaluateScope()) { if (this.runCallback(globalObject, .beforeAll)) |err| { - globalObject.bunVM().onError(globalObject, err); + _ = globalObject.bunVM().uncaughtException(globalObject, err, JSC.JSValue.null); while (i < end) { Jest.runner.?.reportFailure(i + this.test_id_start, source.path.text, tests[i].label, 0, 0, this); i += 1; @@ -1168,7 +1168,7 @@ pub const DescribeScope = struct { if (!skipped) { if (this.runCallback(globalThis, .afterEach)) |err| { - globalThis.bunVM().onError(globalThis, err); + _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); } } @@ -1180,7 +1180,7 @@ pub const DescribeScope = struct { // Run the afterAll callbacks, in reverse order // unless there were no tests for this scope if (this.execCallback(globalThis, .afterAll)) |err| { - globalThis.bunVM().onError(globalThis, err); + _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); } } @@ -1327,7 +1327,7 @@ pub const TestRunnerTask = struct { const label = test_.label; if (this.describe.runCallback(globalThis, .beforeEach)) |err| { - jsc_vm.onError(globalThis, err); + _ = jsc_vm.uncaughtException(globalThis, err, JSC.JSValue.null); Jest.runner.?.reportFailure(test_id, this.source_file_path, label, 0, 0, this.describe); return false; } @@ -1429,7 +1429,7 @@ pub const TestRunnerTask = struct { if (comptime from == .timeout) { const err = this.globalThis.createErrorInstance("Test {} timed out after {d}ms", .{ bun.fmt.quote(test_.label), test_.timeout_millis }); - this.globalThis.bunVM().onError(this.globalThis, err); + _ = this.globalThis.bunVM().uncaughtException(this.globalThis, err, JSC.JSValue.null); } processTestResult(this, this.globalThis, result, test_, test_id, describe); diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index a9e6bd49b235ef..e3a7a1a6b564f3 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -272,7 +272,8 @@ pub const WebWorker = struct { }; if (promise.status(vm.global.vm()) == .Rejected) { - vm.onError(vm.global, promise.result(vm.global.vm())); + // TODO check if we need to do something else here if the promise was handled + _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue()); vm.exit_handler.exit_code = 1; this.exitAndDeinit(); diff --git a/src/bun_js.zig b/src/bun_js.zig index f54e28413efec0..26ecf0996e2614 100644 --- a/src/bun_js.zig +++ b/src/bun_js.zig @@ -274,7 +274,8 @@ pub const Run = struct { if (vm.loadEntryPoint(this.entry_path)) |promise| { if (promise.status(vm.global.vm()) == .Rejected) { - vm.onError(vm.global, promise.result(vm.global.vm())); + // TODO if the exception was handled we may want to continue + _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue()); if (vm.hot_reload != .none) { vm.eventLoop().tick(); @@ -338,7 +339,7 @@ pub const Run = struct { if (this.vm.isWatcherEnabled()) { var prev_promise = this.vm.pending_internal_promise; if (prev_promise.status(vm.global.vm()) == .Rejected) { - vm.onError(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm())); + _ = vm.unhandledRejection(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm()), this.vm.pending_internal_promise.asValue()); } while (true) { @@ -348,7 +349,7 @@ pub const Run = struct { // Report exceptions in hot-reloaded modules if (this.vm.pending_internal_promise.status(vm.global.vm()) == .Rejected and prev_promise != this.vm.pending_internal_promise) { prev_promise = this.vm.pending_internal_promise; - vm.onError(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm())); + _ = vm.unhandledRejection(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm()), this.vm.pending_internal_promise.asValue()); continue; } @@ -359,7 +360,7 @@ pub const Run = struct { if (this.vm.pending_internal_promise.status(vm.global.vm()) == .Rejected and prev_promise != this.vm.pending_internal_promise) { prev_promise = this.vm.pending_internal_promise; - vm.onError(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm())); + _ = vm.unhandledRejection(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm()), this.vm.pending_internal_promise.asValue()); } vm.eventLoop().tickPossiblyForever(); @@ -367,7 +368,7 @@ pub const Run = struct { if (this.vm.pending_internal_promise.status(vm.global.vm()) == .Rejected and prev_promise != this.vm.pending_internal_promise) { prev_promise = this.vm.pending_internal_promise; - vm.onError(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm())); + _ = vm.unhandledRejection(this.vm.global, this.vm.pending_internal_promise.result(vm.global.vm()), this.vm.pending_internal_promise.asValue()); } } else { while (vm.isEventLoopAlive()) { diff --git a/src/cli/test_command.zig b/src/cli/test_command.zig index abd1e287f0628a..2ab6f07918b55a 100644 --- a/src/cli/test_command.zig +++ b/src/cli/test_command.zig @@ -1042,7 +1042,7 @@ pub const TestCommand = struct { switch (promise.status(vm.global.vm())) { .Rejected => { - vm.onError(vm.global, promise.result(vm.global.vm())); + _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue()); reporter.summary.fail += 1; if (reporter.jest.bail == reporter.summary.fail) { @@ -1127,7 +1127,7 @@ pub const TestCommand = struct { if (is_last) { if (jest.Jest.runner != null) { if (jest.DescribeScope.runGlobalCallbacks(vm.global, .afterAll)) |err| { - vm.onError(vm.global, err); + _ = vm.uncaughtException(vm.global, err, JSC.JSValue.null); } } } diff --git a/src/js_ast.zig b/src/js_ast.zig index c4711533d9349d..e4c0fe1f92df75 100644 --- a/src/js_ast.zig +++ b/src/js_ast.zig @@ -7039,7 +7039,7 @@ pub const Macro = struct { var loaded_result = try vm.loadMacroEntryPoint(input_specifier, function_name, specifier, hash); if (loaded_result.status(vm.global.vm()) == JSC.JSPromise.Status.Rejected) { - vm.onError(vm.global, loaded_result.result(vm.global.vm())); + _ = vm.unhandledRejection(vm.global, loaded_result.result(vm.global.vm()), loaded_result.asValue()); vm.disableMacroMode(); return error.MacroLoadError; } @@ -7150,7 +7150,7 @@ pub const Macro = struct { ) MacroError!Expr { switch (comptime tag) { .Error => { - this.macro.vm.onError(this.global, value); + _ = this.macro.vm.uncaughtException(this.global, value, JSC.JSValue.null); return this.caller; }, .Undefined => if (this.is_top_level) @@ -7177,7 +7177,7 @@ pub const Macro = struct { blob_ = resp.*; blob_.?.allocator = null; } else if (value.as(JSC.ResolveMessage) != null or value.as(JSC.BuildMessage) != null) { - this.macro.vm.onError(this.global, value); + _ = this.macro.vm.uncaughtException(this.global, value, JSC.JSValue.null); return error.MacroFailed; } } @@ -7337,15 +7337,11 @@ pub const Macro = struct { return _entry.value_ptr.*; } - var promise_result = JSC.JSValue.zero; - var rejected = false; - if (value.asAnyPromise()) |promise| { - this.macro.vm.waitForPromise(promise); - promise_result = promise.result(this.global.vm()); - rejected = promise.status(this.global.vm()) == .Rejected; - } else { - @panic("Unexpected promise type"); - } + const promise = value.asAnyPromise() orelse @panic("Unexpected promise type"); + + this.macro.vm.waitForPromise(promise); + const promise_result = promise.result(this.global.vm()); + const rejected = promise.status(this.global.vm()) == .Rejected; if (promise_result.isUndefined() and this.is_top_level) { this.is_top_level = false; @@ -7353,7 +7349,7 @@ pub const Macro = struct { } if (rejected or promise_result.isError() or promise_result.isAggregateError(this.global) or promise_result.isException(this.global.vm())) { - this.macro.vm.onError(this.global, promise_result); + _ = this.macro.vm.unhandledRejection(this.global, promise_result, promise.asValue(this.global)); return error.MacroFailed; } this.is_top_level = false; diff --git a/src/napi/napi.zig b/src/napi/napi.zig index 7812a17eed3c62..bb82efc7b579c3 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -1352,7 +1352,7 @@ pub const ThreadSafeFunction = struct { } const err = js_function.call(this.env, &.{}); if (err.isAnyError()) { - this.env.bunVM().onError(this.env, err); + _ = this.env.bunVM().uncaughtException(this.env, err, .null); } }, .c => |cb| { From cc2eeeb4b4e1b59018e377cecd64708d76c74216 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Tue, 7 May 2024 15:52:32 -0700 Subject: [PATCH 02/10] don't exit if exception is handled --- src/bun.js/web_worker.zig | 15 ++++++++------- src/bun_js.zig | 5 ++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index e3a7a1a6b564f3..d6f8ec5e735314 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -272,16 +272,17 @@ pub const WebWorker = struct { }; if (promise.status(vm.global.vm()) == .Rejected) { - // TODO check if we need to do something else here if the promise was handled - _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue()); + const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), .null); - vm.exit_handler.exit_code = 1; - this.exitAndDeinit(); - return; + if (!handled) { + vm.exit_handler.exit_code = 1; + this.exitAndDeinit(); + return; + } + } else { + _ = promise.result(vm.global.vm()); } - _ = promise.result(vm.global.vm()); - this.flushLogs(); log("[{d}] event loop start", .{this.execution_context_id}); WebWorker__dispatchOnline(this.cpp_worker, vm.global); diff --git a/src/bun_js.zig b/src/bun_js.zig index 26ecf0996e2614..8b4ace4f457536 100644 --- a/src/bun_js.zig +++ b/src/bun_js.zig @@ -274,10 +274,9 @@ pub const Run = struct { if (vm.loadEntryPoint(this.entry_path)) |promise| { if (promise.status(vm.global.vm()) == .Rejected) { - // TODO if the exception was handled we may want to continue - _ = vm.unhandledRejection(vm.global, promise.result(vm.global.vm()), promise.asValue()); + const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), .null); - if (vm.hot_reload != .none) { + if (vm.hot_reload != .none or handled) { vm.eventLoop().tick(); vm.eventLoop().tickPossiblyForever(); } else { From 3cd404671626a8d865b26ebdf5be30234bcba900 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Tue, 7 May 2024 16:07:01 -0700 Subject: [PATCH 03/10] implement uncaughtExceptionMonitor --- src/bun.js/bindings/BunProcess.cpp | 24 ++++++++++++++++++++---- src/bun.js/bindings/BunProcess.h | 2 ++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 76c457641eae40..afedd032af1e54 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -751,13 +751,29 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb { auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); + auto& wrapped = process->wrapped(); + MarkedArgumentBuffer args; args.append(exception); args.append(origin); - auto eventType = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); - auto& wrapped = process->wrapped(); - if (wrapped.listenerCount(eventType) > 0) { - wrapped.emit(eventType, args); + + auto uncaughtExceptionMonitor = Identifier::fromString(globalObject->vm(), "uncaughtExceptionMonitor"_s); + if (wrapped.listenerCount(uncaughtExceptionMonitor) > 0) { + wrapped.emit(uncaughtExceptionMonitor, args); + } + + // TODO figure this stuff out + /* + auto capture = process->uncaughtExceptionCaptureCallback(); + if (capture) + capture.call(globalObject, exception, origin); + return true; + } + */ + + auto uncaughtException = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); + if (wrapped.listenerCount(uncaughtException) > 0) { + wrapped.emit(uncaughtException, args); return true; } else { return false; diff --git a/src/bun.js/bindings/BunProcess.h b/src/bun.js/bindings/BunProcess.h index 160827fdf0101a..f76bb821fbf48c 100644 --- a/src/bun.js/bindings/BunProcess.h +++ b/src/bun.js/bindings/BunProcess.h @@ -20,6 +20,7 @@ class Process : public WebCore::JSEventEmitter { LazyProperty m_memoryUsageStructure; LazyProperty m_bindingUV; LazyProperty m_bindingNatives; + // LazyProperty m_uncaughtExceptionCaptureCallback; public: Process(JSC::Structure* structure, WebCore::JSDOMGlobalObject& globalObject, Ref&& impl) @@ -74,6 +75,7 @@ class Process : public WebCore::JSEventEmitter { inline Structure* memoryUsageStructure() { return m_memoryUsageStructure.getInitializedOnMainThread(this); } inline JSObject* bindingUV() { return m_bindingUV.getInitializedOnMainThread(this); } inline JSObject* bindingNatives() { return m_bindingNatives.getInitializedOnMainThread(this); } + // inline JSFunction* uncaughtExceptionCaptureCallback() { return m_uncaughtExceptionCaptureCallback.getInitializedOnMainThread(this); } }; } // namespace Bun \ No newline at end of file From 30ee230696a9ef3ea5394bbd4d0bdeacc598ebb4 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Tue, 7 May 2024 16:48:32 -0700 Subject: [PATCH 04/10] fix crash with runInNewContext --- src/bun.js/bindings/BunProcess.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index afedd032af1e54..bb225ad5930846 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -4,6 +4,7 @@ #include #include #include "JavaScriptCore/JSCJSValue.h" +#include "JavaScriptCore/JSCast.h" #include "ScriptExecutionContext.h" #include "headers-handwritten.h" #include "node_api.h" @@ -749,6 +750,8 @@ void signalHandler(uv_signal_t* signal, int signalNumber) extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue exception, JSC::JSValue origin) { + // TODO do we maybe need to manually get the process object in this case? + if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); auto& wrapped = process->wrapped(); @@ -782,6 +785,8 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb extern "C" int Bun__handleUnhandledRejection(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue reason, JSC::JSValue promise) { + // TODO see above + if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); MarkedArgumentBuffer args; From 273dc4bb83be81bf6330f44b0fab72c07249f343 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Wed, 8 May 2024 13:14:19 -0700 Subject: [PATCH 05/10] implement setUncaughtExceptionCaptureCallback --- src/bun.js/bindings/BunProcess.cpp | 29 +++++++++++++++++++++++------ src/bun.js/bindings/BunProcess.h | 11 +++++++++-- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index bb225ad5930846..e32436a5ac3e02 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -478,6 +478,23 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionExit, return JSC::JSValue::encode(jsUndefined()); } +JSC_DEFINE_HOST_FUNCTION(Process_setUncaughtExceptionCaptureCallback, + (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto throwScope = DECLARE_THROW_SCOPE(globalObject->vm()); + JSValue arg0 = callFrame->argument(0); + if (!arg0.isCallable() && !arg0.isNull()) { + throwTypeError(globalObject, throwScope, "The \"callback\" argument must be callable or null"_s); + return JSC::JSValue::encode(JSC::JSValue {}); + } + auto* zigGlobal = jsDynamicCast(globalObject); + if (UNLIKELY(!zigGlobal)) { + zigGlobal = Bun__getDefaultGlobal(); + } + jsCast(zigGlobal->processObject())->setUncaughtExceptionCaptureCallback(arg0); + return JSC::JSValue::encode(jsUndefined()); +} + extern "C" uint64_t Bun__readOriginTimer(void*); JSC_DEFINE_HOST_FUNCTION(Process_functionHRTime, @@ -765,14 +782,12 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb wrapped.emit(uncaughtExceptionMonitor, args); } - // TODO figure this stuff out - /* - auto capture = process->uncaughtExceptionCaptureCallback(); - if (capture) - capture.call(globalObject, exception, origin); + // if there is an uncaughtExceptionCaptureCallback, call it and consider the exception handled + auto capture = process->getUncaughtExceptionCaptureCallback(); + if (!capture.isEmpty() && !capture.isUndefinedOrNull()) { + call(lexicalGlobalObject, capture, args, ASCIILiteral::fromLiteralUnsafe("uncaughtExceptionCaptureCallback")); return true; } - */ auto uncaughtException = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); if (wrapped.listenerCount(uncaughtException) > 0) { @@ -2089,6 +2104,7 @@ void Process::visitChildrenImpl(JSCell* cell, Visitor& visitor) Process* thisObject = jsCast(cell); ASSERT_GC_OBJECT_INHERITS(thisObject, info()); Base::visitChildren(thisObject, visitor); + visitor.append(thisObject->m_uncaughtExceptionCaptureCallback); thisObject->m_cpuUsageStructure.visit(visitor); thisObject->m_memoryUsageStructure.visit(visitor); thisObject->m_bindingUV.visit(visitor); @@ -2776,6 +2792,7 @@ extern "C" void Process__emitDisconnectEvent(Zig::GlobalObject* global) report constructProcessReportObject PropertyCallback revision constructRevision PropertyCallback setSourceMapsEnabled Process_stubEmptyFunction Function 1 + setUncaughtExceptionCaptureCallback Process_setUncaughtExceptionCaptureCallback Function 1 send constructProcessSend PropertyCallback stderr constructStderr PropertyCallback stdin constructStdin PropertyCallback diff --git a/src/bun.js/bindings/BunProcess.h b/src/bun.js/bindings/BunProcess.h index f76bb821fbf48c..653f4d54a4fb6b 100644 --- a/src/bun.js/bindings/BunProcess.h +++ b/src/bun.js/bindings/BunProcess.h @@ -20,7 +20,7 @@ class Process : public WebCore::JSEventEmitter { LazyProperty m_memoryUsageStructure; LazyProperty m_bindingUV; LazyProperty m_bindingNatives; - // LazyProperty m_uncaughtExceptionCaptureCallback; + WriteBarrier m_uncaughtExceptionCaptureCallback; public: Process(JSC::Structure* structure, WebCore::JSDOMGlobalObject& globalObject, Ref&& impl) @@ -71,11 +71,18 @@ class Process : public WebCore::JSEventEmitter { void finishCreation(JSC::VM& vm); + inline void setUncaughtExceptionCaptureCallback(JSC::JSValue callback) { + m_uncaughtExceptionCaptureCallback.set(vm(), this, callback); + } + + inline JSC::JSValue getUncaughtExceptionCaptureCallback() { + return m_uncaughtExceptionCaptureCallback.get(); + } + inline Structure* cpuUsageStructure() { return m_cpuUsageStructure.getInitializedOnMainThread(this); } inline Structure* memoryUsageStructure() { return m_memoryUsageStructure.getInitializedOnMainThread(this); } inline JSObject* bindingUV() { return m_bindingUV.getInitializedOnMainThread(this); } inline JSObject* bindingNatives() { return m_bindingNatives.getInitializedOnMainThread(this); } - // inline JSFunction* uncaughtExceptionCaptureCallback() { return m_uncaughtExceptionCaptureCallback.getInitializedOnMainThread(this); } }; } // namespace Bun \ No newline at end of file From edc584bb5728754f5f2ef7bdb3766fb0de69ab2c Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Wed, 8 May 2024 13:57:46 -0700 Subject: [PATCH 06/10] tests --- .../process/process-onUncaughtException.js | 29 +++++++++++++++++++ .../process/process-onUnhandledRejection.js | 24 +++++++++++++++ ...rocess-uncaughtExceptionCaptureCallback.js | 29 +++++++++++++++++++ test/js/node/process/process.test.js | 15 ++++++++++ 4 files changed, 97 insertions(+) create mode 100644 test/js/node/process/process-onUncaughtException.js create mode 100644 test/js/node/process/process-onUnhandledRejection.js create mode 100644 test/js/node/process/process-uncaughtExceptionCaptureCallback.js diff --git a/test/js/node/process/process-onUncaughtException.js b/test/js/node/process/process-onUncaughtException.js new file mode 100644 index 00000000000000..f5a1da54d2b034 --- /dev/null +++ b/test/js/node/process/process-onUncaughtException.js @@ -0,0 +1,29 @@ +let monitorCalled = false; + +setTimeout(() => { + // uncaughtExceptionMonitor should be called + if (!monitorCalled) { + process.exit(1); + } + // timeouts should be processed + process.exit(42); +}, 1); + +process.on("uncaughtExceptionMonitor", (err) => { + monitorCalled = true; + if (!err) { + process.exit(1); + } +}); + +process.on("uncaughtException", (err) => { + // there should be an error + if (!err) { + process.exit(1); + } +}) + +throw new Error('error'); + +// this shouldn't be hit even if the exception is caught +process.exit(1); diff --git a/test/js/node/process/process-onUnhandledRejection.js b/test/js/node/process/process-onUnhandledRejection.js new file mode 100644 index 00000000000000..bd7d59a1260f96 --- /dev/null +++ b/test/js/node/process/process-onUnhandledRejection.js @@ -0,0 +1,24 @@ +let unhandledRejectionCalled = false; + +setTimeout(() => { + if (!unhandledRejectionCalled) { + process.exit(1); + } + // timeouts should be processed + process.exit(42); +}, 1); + +let promise; + +process.on("unhandledRejection", (err, promise) => { + unhandledRejectionCalled = true; + // there should be an error + if (!err) { + process.exit(1); + } + if (promise !== promise) { + process.exit(1); + } +}); + +promise = Promise.reject(new Error('error')); diff --git a/test/js/node/process/process-uncaughtExceptionCaptureCallback.js b/test/js/node/process/process-uncaughtExceptionCaptureCallback.js new file mode 100644 index 00000000000000..085a1ebd6ec556 --- /dev/null +++ b/test/js/node/process/process-uncaughtExceptionCaptureCallback.js @@ -0,0 +1,29 @@ +let monitorCalled = false; + +setTimeout(() => { + // uncaughtExceptionMonitor should be called + if (!monitorCalled) { + process.exit(1); + } + // timeouts should be processed + process.exit(42); +}, 1); + +process.on("uncaughtExceptionMonitor", (err) => { + monitorCalled = true; + if (!err) { + process.exit(1); + } +}); + +process.setUncaughtExceptionCaptureCallback((err) => { + // there should be an error + if (!err) { + process.exit(1); + } +}) + +throw new Error('error'); + +// this shouldn't be hit even if the exception is caught +process.exit(1); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index faa1a9fea8f145..5aa1713705ebef 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -572,3 +572,18 @@ if (isWindows) { expect(() => Object.getOwnPropertyDescriptors(process.env)).not.toThrow(); }); } + +it("catches exceptions with process.setUncaughtExceptionCaptureCallback", async () => { + const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-uncaughtExceptionCaptureCallback.js")]); + expect(await proc.exited).toBe(42); +}); + +it("catches exceptions with process.on('uncaughtException', fn)", async () => { + const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUncaughtException.js")]); + expect(await proc.exited).toBe(42); +}); + +it("catches exceptions with process.on('unhandledRejection', fn)", async () => { + const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUnhandledRejection.js")]); + expect(await proc.exited).toBe(42); +}); From ef87d0854c02bcdabcc4b563e1639c056c3b6fe7 Mon Sep 17 00:00:00 2001 From: gvilums Date: Wed, 8 May 2024 21:07:00 +0000 Subject: [PATCH 07/10] Apply formatting changes --- test/js/node/process/process-onUncaughtException.js | 8 ++++---- test/js/node/process/process-onUnhandledRejection.js | 2 +- .../process/process-uncaughtExceptionCaptureCallback.js | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/js/node/process/process-onUncaughtException.js b/test/js/node/process/process-onUncaughtException.js index f5a1da54d2b034..8f85e0aa4c4fc7 100644 --- a/test/js/node/process/process-onUncaughtException.js +++ b/test/js/node/process/process-onUncaughtException.js @@ -9,21 +9,21 @@ setTimeout(() => { process.exit(42); }, 1); -process.on("uncaughtExceptionMonitor", (err) => { +process.on("uncaughtExceptionMonitor", err => { monitorCalled = true; if (!err) { process.exit(1); } }); -process.on("uncaughtException", (err) => { +process.on("uncaughtException", err => { // there should be an error if (!err) { process.exit(1); } -}) +}); -throw new Error('error'); +throw new Error("error"); // this shouldn't be hit even if the exception is caught process.exit(1); diff --git a/test/js/node/process/process-onUnhandledRejection.js b/test/js/node/process/process-onUnhandledRejection.js index bd7d59a1260f96..bb7830721bbade 100644 --- a/test/js/node/process/process-onUnhandledRejection.js +++ b/test/js/node/process/process-onUnhandledRejection.js @@ -21,4 +21,4 @@ process.on("unhandledRejection", (err, promise) => { } }); -promise = Promise.reject(new Error('error')); +promise = Promise.reject(new Error("error")); diff --git a/test/js/node/process/process-uncaughtExceptionCaptureCallback.js b/test/js/node/process/process-uncaughtExceptionCaptureCallback.js index 085a1ebd6ec556..55b304302c481c 100644 --- a/test/js/node/process/process-uncaughtExceptionCaptureCallback.js +++ b/test/js/node/process/process-uncaughtExceptionCaptureCallback.js @@ -9,21 +9,21 @@ setTimeout(() => { process.exit(42); }, 1); -process.on("uncaughtExceptionMonitor", (err) => { +process.on("uncaughtExceptionMonitor", err => { monitorCalled = true; if (!err) { process.exit(1); } }); -process.setUncaughtExceptionCaptureCallback((err) => { +process.setUncaughtExceptionCaptureCallback(err => { // there should be an error if (!err) { process.exit(1); } -}) +}); -throw new Error('error'); +throw new Error("error"); // this shouldn't be hit even if the exception is caught process.exit(1); From 6cdf0be6f1bff4f81d6dc57b2018a3021ec2eaf0 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums <=> Date: Wed, 8 May 2024 15:55:17 -0700 Subject: [PATCH 08/10] abort on throw in handler --- src/bun.js/api/BunObject.zig | 6 +-- src/bun.js/api/bun/socket.zig | 4 +- src/bun.js/api/bun/udp_socket.zig | 4 +- src/bun.js/api/server.zig | 6 +-- src/bun.js/bindings/BunProcess.cpp | 40 +++++++++++++------ src/bun.js/event_loop.zig | 2 +- src/bun.js/javascript.zig | 23 ++++++++--- src/bun.js/node/node_fs_stat_watcher.zig | 4 +- src/bun.js/node/node_fs_watcher.zig | 2 +- src/bun.js/test/jest.zig | 20 +++++----- src/bun.js/web_worker.zig | 2 +- src/bun_js.zig | 2 +- src/cli/test_command.zig | 2 +- src/js_ast.zig | 4 +- src/napi/napi.zig | 2 +- .../process-onUncaughtExceptionAbort.js | 5 +++ ...s-uncaughtExceptionCaptureCallbackAbort.js | 5 +++ test/js/node/process/process.test.js | 16 ++++++++ 18 files changed, 102 insertions(+), 47 deletions(-) create mode 100644 test/js/node/process/process-onUncaughtExceptionAbort.js create mode 100644 test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js diff --git a/src/bun.js/api/BunObject.zig b/src/bun.js/api/BunObject.zig index d124278c2d7e2d..0c552c2c8d4d92 100644 --- a/src/bun.js/api/BunObject.zig +++ b/src/bun.js/api/BunObject.zig @@ -3685,7 +3685,7 @@ pub const Timer = struct { } var this = args.ptr[1].asPtr(CallbackJob); - _ = globalThis.bunVM().uncaughtException(globalThis, args.ptr[0], .null); + _ = globalThis.bunVM().uncaughtException(globalThis, args.ptr[0], true); this.deinit(); return JSValue.jsUndefined(); } @@ -3787,7 +3787,7 @@ pub const Timer = struct { } if (result.isAnyError()) { - _ = vm.uncaughtException(globalThis, result, JSC.JSValue.null); + _ = vm.uncaughtException(globalThis, result, false); this.deinit(); return; } @@ -5265,7 +5265,7 @@ pub const EnvironmentVariables = struct { }; export fn Bun__reportError(globalObject: *JSGlobalObject, err: JSC.JSValue) void { - _ = JSC.VirtualMachine.get().uncaughtException(globalObject, err, .null); + _ = JSC.VirtualMachine.get().uncaughtException(globalObject, err, false); } comptime { diff --git a/src/bun.js/api/bun/socket.zig b/src/bun.js/api/bun/socket.zig index 86ca8fc2661b6a..0f128ba3145775 100644 --- a/src/bun.js/api/bun/socket.zig +++ b/src/bun.js/api/bun/socket.zig @@ -236,14 +236,14 @@ const Handlers = struct { const onError = this.onError; if (onError == .zero) { if (err.len > 0) - _ = this.vm.uncaughtException(this.globalObject, err[0], JSC.JSValue.null); + _ = this.vm.uncaughtException(this.globalObject, err[0], false); return false; } const result = onError.callWithThis(this.globalObject, thisValue, err); if (result.isAnyError()) { - _ = this.vm.uncaughtException(this.globalObject, result, JSC.JSValue.null); + _ = this.vm.uncaughtException(this.globalObject, result, false); } return true; diff --git a/src/bun.js/api/bun/udp_socket.zig b/src/bun.js/api/bun/udp_socket.zig index 5418d775df2272..de0dbe29bba2fb 100644 --- a/src/bun.js/api/bun/udp_socket.zig +++ b/src/bun.js/api/bun/udp_socket.zig @@ -359,14 +359,14 @@ pub const UDPSocket = struct { if (callback == .zero) { if (err.len > 0) - _ = vm.uncaughtException(globalThis, err[0], JSC.JSValue.null); + _ = vm.uncaughtException(globalThis, err[0], false); return false; } const result = callback.callWithThis(globalThis, thisValue, err); if (result.isAnyError()) { - _ = vm.uncaughtException(globalThis, result, JSC.JSValue.null); + _ = vm.uncaughtException(globalThis, result, false); } return true; diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index 5709ddb8d64e4e..89a4ffc41d6775 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -3930,7 +3930,7 @@ pub const ServerWebSocket = struct { } if (error_handler.isEmptyOrUndefinedOrNull()) { - _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); + _ = vm.uncaughtException(globalObject, err_value, false); } else { const corky = [_]JSValue{err_value}; corker.args = &corky; @@ -3997,7 +3997,7 @@ pub const ServerWebSocket = struct { if (result.toError()) |err_value| { if (this.handler.onError.isEmptyOrUndefinedOrNull()) { - _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); + _ = vm.uncaughtException(globalObject, err_value, false); } else { const args = [_]JSValue{err_value}; corker.args = &args; @@ -4049,7 +4049,7 @@ pub const ServerWebSocket = struct { if (result.toError()) |err_value| { if (this.handler.onError.isEmptyOrUndefinedOrNull()) { - _ = vm.uncaughtException(globalObject, err_value, JSC.JSValue.null); + _ = vm.uncaughtException(globalObject, err_value, false); } else { const args = [_]JSValue{err_value}; corker.args = &args; diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index e32436a5ac3e02..8cc6d132a886a0 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -3,8 +3,11 @@ #include #include #include +#include "JavaScriptCore/CatchScope.h" #include "JavaScriptCore/JSCJSValue.h" #include "JavaScriptCore/JSCast.h" +#include "JavaScriptCore/JSString.h" +#include "JavaScriptCore/Protect.h" #include "ScriptExecutionContext.h" #include "headers-handwritten.h" #include "node_api.h" @@ -22,6 +25,7 @@ #include "wtf-bindings.h" #include "ProcessBindingTTYWrap.h" +#include "wtf/text/ASCIILiteral.h" #ifndef WIN32 #include @@ -765,42 +769,54 @@ void signalHandler(uv_signal_t* signal, int signalNumber) }); }; -extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue exception, JSC::JSValue origin) +extern "C" void Bun__logUnhandledException(JSC::EncodedJSValue exception); + +extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue exception, int isRejection) { - // TODO do we maybe need to manually get the process object in this case? if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); auto& wrapped = process->wrapped(); + auto& vm = globalObject->vm(); MarkedArgumentBuffer args; args.append(exception); - args.append(origin); + if (isRejection) { + args.append(jsString(vm, WTF::StringView::fromLatin1("unhandledRejection"))); + } else { + args.append(jsString(vm, WTF::StringView::fromLatin1("uncaughtException"))); + } + auto uncaughtExceptionMonitor = Identifier::fromString(globalObject->vm(), "uncaughtExceptionMonitor"_s); if (wrapped.listenerCount(uncaughtExceptionMonitor) > 0) { wrapped.emit(uncaughtExceptionMonitor, args); } + auto uncaughtExceptionIdent = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); + // if there is an uncaughtExceptionCaptureCallback, call it and consider the exception handled auto capture = process->getUncaughtExceptionCaptureCallback(); if (!capture.isEmpty() && !capture.isUndefinedOrNull()) { - call(lexicalGlobalObject, capture, args, ASCIILiteral::fromLiteralUnsafe("uncaughtExceptionCaptureCallback")); - return true; - } - - auto uncaughtException = Identifier::fromString(globalObject->vm(), "uncaughtException"_s); - if (wrapped.listenerCount(uncaughtException) > 0) { - wrapped.emit(uncaughtException, args); - return true; + auto scope = DECLARE_CATCH_SCOPE(vm); + (void)call(lexicalGlobalObject, capture, args, "uncaughtExceptionCaptureCallback"_s); + if (auto ex = scope.exception()) { + scope.clearException(); + // if an exception is thrown in the uncaughtException handler, we abort + Bun__logUnhandledException(JSValue::encode(JSValue(ex))); + Bun__Process__exit(lexicalGlobalObject, 1); + } + } else if (wrapped.listenerCount(uncaughtExceptionIdent) > 0) { + wrapped.emit(uncaughtExceptionIdent, args); } else { return false; } + + return true; } extern "C" int Bun__handleUnhandledRejection(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue reason, JSC::JSValue promise) { - // TODO see above if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index 4499058a896249..9d109be64d433b 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -855,7 +855,7 @@ pub const EventLoop = struct { const result = callback.callWithThis(globalObject, thisValue, arguments); if (result.toError()) |err| { - _ = this.virtual_machine.uncaughtException(globalObject, err, .null); + _ = this.virtual_machine.uncaughtException(globalObject, err, false); } } diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index a31f875d2a9d78..0273984f328e00 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -351,7 +351,7 @@ pub export fn Bun__reportUnhandledError(globalObject: *JSGlobalObject, value: JS // This JSGlobalObject might not be the main script execution context // See the crash in https://github.com/oven-sh/bun/issues/9778 const jsc_vm = JSC.VirtualMachine.get(); - _ = jsc_vm.uncaughtException(globalObject, value, .undefined); + _ = jsc_vm.uncaughtException(globalObject, value, false); return JSC.JSValue.jsUndefined(); } @@ -608,6 +608,7 @@ pub const VirtualMachine = struct { onUnhandledRejection: *const OnUnhandledRejection = defaultOnUnhandledRejection, onUnhandledRejectionCtx: ?*anyopaque = null, unhandled_error_counter: usize = 0, + is_handling_uncaught_exception: bool = false, modules: ModuleLoader.AsyncModule.Queue = .{}, aggressive_garbage_collection: GCLevel = GCLevel.none, @@ -821,8 +822,9 @@ pub const VirtualMachine = struct { } } - extern fn Bun__handleUncaughtException(*JSC.JSGlobalObject, err: JSC.JSValue, origin: JSC.JSValue) c_int; + extern fn Bun__handleUncaughtException(*JSC.JSGlobalObject, err: JSC.JSValue, is_rejection: c_int) c_int; extern fn Bun__handleUnhandledRejection(*JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) c_int; + extern fn Bun__Process__exit(*JSC.JSGlobalObject, code: c_int) noreturn; pub fn unhandledRejection(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) bool { const handled = Bun__handleUnhandledRejection(globalObject, reason, promise) > 0; @@ -833,8 +835,15 @@ pub const VirtualMachine = struct { return handled; } - pub fn uncaughtException(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, err: JSC.JSValue, origin: JSC.JSValue) bool { - const handled = Bun__handleUncaughtException(globalObject, err, origin) > 0; + pub fn uncaughtException(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, err: JSC.JSValue, is_rejection: bool) bool { + if (this.is_handling_uncaught_exception) { + this.runErrorHandler(err, null); + Bun__Process__exit(globalObject, 1); + @panic("Uncaught exception while handling uncaught exception"); + } + this.is_handling_uncaught_exception = true; + defer this.is_handling_uncaught_exception = false; + const handled = Bun__handleUncaughtException(globalObject, err, if (is_rejection) 1 else 0) > 0; if (!handled) { // TODO maybe we want a separate code path for uncaught exceptions this.unhandled_error_counter += 1; @@ -2181,6 +2190,10 @@ pub const VirtualMachine = struct { } } + export fn Bun__logUnhandledException(exception: JSC.JSValue) void { + get().runErrorHandler(exception, null); + } + pub fn clearEntryPoint( this: *VirtualMachine, ) void { @@ -2627,7 +2640,7 @@ pub const VirtualMachine = struct { pub fn reportUncaughtException(globalObject: *JSGlobalObject, exception: *JSC.Exception) JSValue { var jsc_vm = globalObject.bunVM(); - _ = jsc_vm.uncaughtException(globalObject, exception.value(), JSC.JSValue.jsUndefined()); + _ = jsc_vm.uncaughtException(globalObject, exception.value(), false); return JSC.JSValue.jsUndefined(); } diff --git a/src/bun.js/node/node_fs_stat_watcher.zig b/src/bun.js/node/node_fs_stat_watcher.zig index e0bbd78fdb7264..418fd9e3d41b52 100644 --- a/src/bun.js/node/node_fs_stat_watcher.zig +++ b/src/bun.js/node/node_fs_stat_watcher.zig @@ -385,7 +385,7 @@ pub const StatWatcher = struct { const vm = this.globalThis.bunVM(); if (result.isAnyError()) { - _ = vm.uncaughtException(this.globalThis, result, JSC.JSValue.null); + _ = vm.uncaughtException(this.globalThis, result, false); } vm.rareData().nodeFSStatWatcherScheduler(vm).append(this); @@ -421,7 +421,7 @@ pub const StatWatcher = struct { ); if (result.isAnyError()) { const vm = this.globalThis.bunVM(); - _ = vm.uncaughtException(this.globalThis, result, JSC.JSValue.null); + _ = vm.uncaughtException(this.globalThis, result, false); } } diff --git a/src/bun.js/node/node_fs_watcher.zig b/src/bun.js/node/node_fs_watcher.zig index d4061c43c7e5c0..9dce08664e38b3 100644 --- a/src/bun.js/node/node_fs_watcher.zig +++ b/src/bun.js/node/node_fs_watcher.zig @@ -606,7 +606,7 @@ pub const FSWatcher = struct { ); if (err.toError()) |value| { - _ = JSC.VirtualMachine.get().uncaughtException(globalObject, value, JSC.JSValue.null); + _ = JSC.VirtualMachine.get().uncaughtException(globalObject, value, false); } } diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig index 73160a957618b3..976d2c9f744dc7 100644 --- a/src/bun.js/test/jest.zig +++ b/src/bun.js/test/jest.zig @@ -602,7 +602,7 @@ pub const TestScope = struct { debug("onReject", .{}); const arguments = callframe.arguments(2); const err = arguments.ptr[0]; - _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); + _ = globalThis.bunVM().uncaughtException(globalThis, err, true); var task: *TestRunnerTask = arguments.ptr[1].asPromisePtr(TestRunnerTask); task.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .promise); globalThis.bunVM().autoGarbageCollect(); @@ -637,7 +637,7 @@ pub const TestScope = struct { task.handleResult(.{ .pass = expect.active_test_expectation_counter.actual }, .callback); } else { debug("done(err)", .{}); - _ = globalThis.bunVM().uncaughtException(globalThis, err, JSC.JSValue.null); + _ = globalThis.bunVM().uncaughtException(globalThis, err, true); task.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .callback); } } else { @@ -696,7 +696,7 @@ pub const TestScope = struct { initial_value = callJSFunctionForTestRunner(vm, vm.global, this.func, this.func_arg); if (initial_value.isAnyError()) { - _ = vm.uncaughtException(vm.global, initial_value, JSC.JSValue.null); + _ = vm.uncaughtException(vm.global, initial_value, true); if (this.tag == .todo) { return .{ .todo = {} }; @@ -893,7 +893,7 @@ pub const DescribeScope = struct { if (args.len > 0) { const err = args.ptr[0]; if (!err.isEmptyOrUndefinedOrNull()) { - _ = ctx.bunVM().uncaughtException(ctx.bunVM().global, err, JSC.JSValue.null); + _ = ctx.bunVM().uncaughtException(ctx.bunVM().global, err, true); } } scope.done = true; @@ -1095,7 +1095,7 @@ pub const DescribeScope = struct { }, } } else if (result.toError()) |err| { - _ = globalObject.bunVM().uncaughtException(globalObject, err, .null); + _ = globalObject.bunVM().uncaughtException(globalObject, err, true); return .undefined; } } @@ -1123,7 +1123,7 @@ pub const DescribeScope = struct { if (this.shouldEvaluateScope()) { if (this.runCallback(globalObject, .beforeAll)) |err| { - _ = globalObject.bunVM().uncaughtException(globalObject, err, JSC.JSValue.null); + _ = globalObject.bunVM().uncaughtException(globalObject, err, true); while (i < end) { Jest.runner.?.reportFailure(i + this.test_id_start, source.path.text, tests[i].label, 0, 0, this); i += 1; @@ -1168,7 +1168,7 @@ pub const DescribeScope = struct { if (!skipped) { if (this.runCallback(globalThis, .afterEach)) |err| { - _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); + _ = globalThis.bunVM().uncaughtException(globalThis, err, true); } } @@ -1180,7 +1180,7 @@ pub const DescribeScope = struct { // Run the afterAll callbacks, in reverse order // unless there were no tests for this scope if (this.execCallback(globalThis, .afterAll)) |err| { - _ = globalThis.bunVM().uncaughtException(globalThis, err, .null); + _ = globalThis.bunVM().uncaughtException(globalThis, err, true); } } @@ -1327,7 +1327,7 @@ pub const TestRunnerTask = struct { const label = test_.label; if (this.describe.runCallback(globalThis, .beforeEach)) |err| { - _ = jsc_vm.uncaughtException(globalThis, err, JSC.JSValue.null); + _ = jsc_vm.uncaughtException(globalThis, err, true); Jest.runner.?.reportFailure(test_id, this.source_file_path, label, 0, 0, this.describe); return false; } @@ -1429,7 +1429,7 @@ pub const TestRunnerTask = struct { if (comptime from == .timeout) { const err = this.globalThis.createErrorInstance("Test {} timed out after {d}ms", .{ bun.fmt.quote(test_.label), test_.timeout_millis }); - _ = this.globalThis.bunVM().uncaughtException(this.globalThis, err, JSC.JSValue.null); + _ = this.globalThis.bunVM().uncaughtException(this.globalThis, err, true); } processTestResult(this, this.globalThis, result, test_, test_id, describe); diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index d6f8ec5e735314..ffbbb19d4a268d 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -272,7 +272,7 @@ pub const WebWorker = struct { }; if (promise.status(vm.global.vm()) == .Rejected) { - const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), .null); + const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), true); if (!handled) { vm.exit_handler.exit_code = 1; diff --git a/src/bun_js.zig b/src/bun_js.zig index 8b4ace4f457536..8ccb36c6be8ed6 100644 --- a/src/bun_js.zig +++ b/src/bun_js.zig @@ -274,7 +274,7 @@ pub const Run = struct { if (vm.loadEntryPoint(this.entry_path)) |promise| { if (promise.status(vm.global.vm()) == .Rejected) { - const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), .null); + const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), true); if (vm.hot_reload != .none or handled) { vm.eventLoop().tick(); diff --git a/src/cli/test_command.zig b/src/cli/test_command.zig index 2ab6f07918b55a..797c0c1c4ca189 100644 --- a/src/cli/test_command.zig +++ b/src/cli/test_command.zig @@ -1127,7 +1127,7 @@ pub const TestCommand = struct { if (is_last) { if (jest.Jest.runner != null) { if (jest.DescribeScope.runGlobalCallbacks(vm.global, .afterAll)) |err| { - _ = vm.uncaughtException(vm.global, err, JSC.JSValue.null); + _ = vm.uncaughtException(vm.global, err, true); } } } diff --git a/src/js_ast.zig b/src/js_ast.zig index e4c0fe1f92df75..6bf4496fceabb5 100644 --- a/src/js_ast.zig +++ b/src/js_ast.zig @@ -7150,7 +7150,7 @@ pub const Macro = struct { ) MacroError!Expr { switch (comptime tag) { .Error => { - _ = this.macro.vm.uncaughtException(this.global, value, JSC.JSValue.null); + _ = this.macro.vm.uncaughtException(this.global, value, false); return this.caller; }, .Undefined => if (this.is_top_level) @@ -7177,7 +7177,7 @@ pub const Macro = struct { blob_ = resp.*; blob_.?.allocator = null; } else if (value.as(JSC.ResolveMessage) != null or value.as(JSC.BuildMessage) != null) { - _ = this.macro.vm.uncaughtException(this.global, value, JSC.JSValue.null); + _ = this.macro.vm.uncaughtException(this.global, value, false); return error.MacroFailed; } } diff --git a/src/napi/napi.zig b/src/napi/napi.zig index bb82efc7b579c3..bcde7745c7cdb6 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -1352,7 +1352,7 @@ pub const ThreadSafeFunction = struct { } const err = js_function.call(this.env, &.{}); if (err.isAnyError()) { - _ = this.env.bunVM().uncaughtException(this.env, err, .null); + _ = this.env.bunVM().uncaughtException(this.env, err, false); } }, .c => |cb| { diff --git a/test/js/node/process/process-onUncaughtExceptionAbort.js b/test/js/node/process/process-onUncaughtExceptionAbort.js new file mode 100644 index 00000000000000..973a5fab8337f7 --- /dev/null +++ b/test/js/node/process/process-onUncaughtExceptionAbort.js @@ -0,0 +1,5 @@ +process.on("uncaughtException", (err) => { + throw new Error('bar') +}) + +throw new Error('foo'); diff --git a/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js b/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js new file mode 100644 index 00000000000000..bb5ca24a2599c4 --- /dev/null +++ b/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js @@ -0,0 +1,5 @@ +process.setUncaughtExceptionCaptureCallback((err) => { + throw new Error('bar') +}) + +throw new Error('foo'); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 5aa1713705ebef..d0d34803e5e87a 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -587,3 +587,19 @@ it("catches exceptions with process.on('unhandledRejection', fn)", async () => { const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUnhandledRejection.js")]); expect(await proc.exited).toBe(42); }); + +it("aborts when the uncaughtException handler throws", async () => { + const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUncaughtExceptionAbort.js")], { + stderr: "pipe" + }); + expect(await proc.exited).toBe(1); + expect(await new Response(proc.stderr).text()).toContain("bar"); +}); + +it("aborts when the uncaughtExceptionCaptureCallback throws", async () => { + const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-uncaughtExceptionCaptureCallbackAbort.js")], { + stderr: "pipe", + }); + expect(await proc.exited).toBe(1); + expect(await new Response(proc.stderr).text()).toContain("bar"); +}); From a7cb64646dcab4a628b5814ee14e4234f640789f Mon Sep 17 00:00:00 2001 From: gvilums Date: Wed, 8 May 2024 23:03:28 +0000 Subject: [PATCH 09/10] Apply formatting changes --- test/cli/install/bun-install.test.ts | 2 -- test/js/node/process/process-onUncaughtExceptionAbort.js | 8 ++++---- .../process-uncaughtExceptionCaptureCallbackAbort.js | 8 ++++---- test/js/node/process/process.test.js | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 074f389a728440..bbdb8afa5ac722 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -276,7 +276,6 @@ it("should work when moving workspace packages", async () => { }, }); - await Bun.$`${bunExe()} i`.env(bunEnv).cwd(package_dir); await Bun.$/* sh */ ` @@ -349,7 +348,6 @@ it("should work when renaming a single workspace package", async () => { }, }); - await Bun.$`${bunExe()} i`.env(bunEnv).cwd(package_dir); await Bun.$/* sh */ ` diff --git a/test/js/node/process/process-onUncaughtExceptionAbort.js b/test/js/node/process/process-onUncaughtExceptionAbort.js index 973a5fab8337f7..713d666fd2b004 100644 --- a/test/js/node/process/process-onUncaughtExceptionAbort.js +++ b/test/js/node/process/process-onUncaughtExceptionAbort.js @@ -1,5 +1,5 @@ -process.on("uncaughtException", (err) => { - throw new Error('bar') -}) +process.on("uncaughtException", err => { + throw new Error("bar"); +}); -throw new Error('foo'); +throw new Error("foo"); diff --git a/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js b/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js index bb5ca24a2599c4..6c3f27d9f41747 100644 --- a/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js +++ b/test/js/node/process/process-uncaughtExceptionCaptureCallbackAbort.js @@ -1,5 +1,5 @@ -process.setUncaughtExceptionCaptureCallback((err) => { - throw new Error('bar') -}) +process.setUncaughtExceptionCaptureCallback(err => { + throw new Error("bar"); +}); -throw new Error('foo'); +throw new Error("foo"); diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index d0d34803e5e87a..4ac6d45fe059d0 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -590,7 +590,7 @@ it("catches exceptions with process.on('unhandledRejection', fn)", async () => { it("aborts when the uncaughtException handler throws", async () => { const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUncaughtExceptionAbort.js")], { - stderr: "pipe" + stderr: "pipe", }); expect(await proc.exited).toBe(1); expect(await new Response(proc.stderr).text()).toContain("bar"); From 18b41f78ed6a55461880ffe4161df6793b91f49d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 8 May 2024 16:52:23 -0700 Subject: [PATCH 10/10] Disable in tests --- src/bun.js/bindings/BunProcess.cpp | 19 +++++++------------ src/bun.js/javascript.zig | 12 ++++++++++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 8cc6d132a886a0..18444f371d42e0 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -490,7 +490,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_setUncaughtExceptionCaptureCallback, if (!arg0.isCallable() && !arg0.isNull()) { throwTypeError(globalObject, throwScope, "The \"callback\" argument must be callable or null"_s); return JSC::JSValue::encode(JSC::JSValue {}); - } + } auto* zigGlobal = jsDynamicCast(globalObject); if (UNLIKELY(!zigGlobal)) { zigGlobal = Bun__getDefaultGlobal(); @@ -773,7 +773,8 @@ extern "C" void Bun__logUnhandledException(JSC::EncodedJSValue exception); extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue exception, int isRejection) { - if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; + if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) + return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); auto& wrapped = process->wrapped(); @@ -782,12 +783,11 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb MarkedArgumentBuffer args; args.append(exception); if (isRejection) { - args.append(jsString(vm, WTF::StringView::fromLatin1("unhandledRejection"))); + args.append(jsString(vm, String("unhandledRejection"_s))); } else { - args.append(jsString(vm, WTF::StringView::fromLatin1("uncaughtException"))); + args.append(jsString(vm, String("uncaughtException"_s))); } - auto uncaughtExceptionMonitor = Identifier::fromString(globalObject->vm(), "uncaughtExceptionMonitor"_s); if (wrapped.listenerCount(uncaughtExceptionMonitor) > 0) { wrapped.emit(uncaughtExceptionMonitor, args); @@ -817,7 +817,8 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb extern "C" int Bun__handleUnhandledRejection(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue reason, JSC::JSValue promise) { - if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) return false; + if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info())) + return false; auto* globalObject = jsCast(lexicalGlobalObject); auto* process = jsCast(globalObject->processObject()); MarkedArgumentBuffer args; @@ -854,12 +855,6 @@ static void onDidChangeListeners(EventEmitter& eventEmitter, const Identifier& e return; } - if (eventName.string() == "uncaughtException"_s) { - if (isAdded) { - - } - } - // Signal Handlers loadSignalNumberMap(); static std::once_flag signalNumberToNameMapOnceFlag; diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 0273984f328e00..36b028ab35af10 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -827,6 +827,12 @@ pub const VirtualMachine = struct { extern fn Bun__Process__exit(*JSC.JSGlobalObject, code: c_int) noreturn; pub fn unhandledRejection(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) bool { + if (isBunTest) { + this.unhandled_error_counter += 1; + this.onUnhandledRejection(this, globalObject, reason); + return true; + } + const handled = Bun__handleUnhandledRejection(globalObject, reason, promise) > 0; if (!handled) { this.unhandled_error_counter += 1; @@ -836,6 +842,12 @@ pub const VirtualMachine = struct { } pub fn uncaughtException(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, err: JSC.JSValue, is_rejection: bool) bool { + if (isBunTest) { + this.unhandled_error_counter += 1; + this.onUnhandledRejection(this, globalObject, err); + return true; + } + if (this.is_handling_uncaught_exception) { this.runErrorHandler(err, null); Bun__Process__exit(globalObject, 1);