Skip to content

Commit

Permalink
Make unhandled exceptions in Bun.serve() within a test() or expect() …
Browse files Browse the repository at this point in the history 
…call fail the test like any other exception (#11141)
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner committed May 18, 2024
1 parent 4e714ae commit 16920a5
Show file tree
Hide file tree
Showing 7 changed files with 248 additions and 126 deletions.
81 changes: 36 additions & 45 deletions src/bun.js/api/server.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3031,11 +3031,14 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
}

fn finishRunningErrorHandler(this: *RequestContext, value: JSC.JSValue, status: u16) void {
var vm = this.server.vm;
var exception_list: std.ArrayList(Api.JsException) = std.ArrayList(Api.JsException).init(this.allocator);
defer exception_list.deinit();
var vm: *JSC.VirtualMachine = this.server.vm;
if (comptime debug_mode) {
vm.runErrorHandler(value, &exception_list);
var exception_list: std.ArrayList(Api.JsException) = std.ArrayList(Api.JsException).init(this.allocator);
defer exception_list.deinit();
const prev_exception_list = vm.onUnhandledRejectionExceptionList;
vm.onUnhandledRejectionExceptionList = &exception_list;
vm.onUnhandledRejection(vm, this.server.globalThis, value);
vm.onUnhandledRejectionExceptionList = prev_exception_list;

this.renderDefaultError(
vm.log,
Expand All @@ -3045,8 +3048,9 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
.{ @as(string, @tagName(this.method)), this.ensurePathname() },
);
} else {
if (status != 404)
vm.runErrorHandler(value, &exception_list);
if (status != 404) {
vm.onUnhandledRejection(vm, this.server.globalThis, value);
}
this.renderProductionError(status);
}

Expand Down Expand Up @@ -3525,6 +3529,19 @@ pub const WebSocketServer = struct {
publish_to_self: bool = false,
} = .{},

pub fn runErrorCallback(this: *const Handler, vm: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, error_value: JSC.JSValue) void {
const onError = this.onError;
if (!onError.isEmptyOrUndefinedOrNull()) {
const err_ret = onError.callWithThis(globalObject, .undefined, &.{error_value});
if (err_ret.toError()) |actual_err| {
_ = vm.uncaughtException(globalObject, actual_err, false);
}
return;
}

_ = vm.uncaughtException(globalObject, error_value, false);
}

pub fn fromJS(globalObject: *JSC.JSGlobalObject, object: JSC.JSValue) ?Handler {
const vm = globalObject.vm();
var handler = Handler{ .globalObject = globalObject, .vm = VirtualMachine.get() };
Expand Down Expand Up @@ -3912,7 +3929,6 @@ pub const ServerWebSocket = struct {
.globalObject = globalObject,
.callback = onOpenHandler,
};
const error_handler = handler.onError;
ws.cork(&corker, Corker.run);
const result = corker.result;
this.flags.opened = true;
Expand All @@ -3929,16 +3945,7 @@ pub const ServerWebSocket = struct {
this_value.unprotect();
}

if (error_handler.isEmptyOrUndefinedOrNull()) {
_ = vm.uncaughtException(globalObject, err_value, false);
} else {
const corky = [_]JSValue{err_value};
corker.args = &corky;
corker.callback = error_handler;
corker.this_value = .zero;
corker.result = .zero;
corker.run();
}
handler.runErrorCallback(vm, globalObject, err_value);
}
}

Expand Down Expand Up @@ -3996,16 +4003,7 @@ pub const ServerWebSocket = struct {
if (result.isEmptyOrUndefinedOrNull()) return;

if (result.toError()) |err_value| {
if (this.handler.onError.isEmptyOrUndefinedOrNull()) {
_ = vm.uncaughtException(globalObject, err_value, false);
} else {
const args = [_]JSValue{err_value};
corker.args = &args;
corker.callback = this.handler.onError;
corker.this_value = .zero;
corker.result = .zero;
corker.run();
}
this.handler.runErrorCallback(vm, globalObject, err_value);
return;
}

Expand Down Expand Up @@ -4048,16 +4046,7 @@ pub const ServerWebSocket = struct {
const result = corker.result;

if (result.toError()) |err_value| {
if (this.handler.onError.isEmptyOrUndefinedOrNull()) {
_ = vm.uncaughtException(globalObject, err_value, false);
} else {
const args = [_]JSValue{err_value};
corker.args = &args;
corker.callback = this.handler.onError;
corker.this_value = .zero;
corker.result = .zero;
corker.run();
}
handler.runErrorCallback(vm, globalObject, err_value);
}
}
}
Expand All @@ -4084,7 +4073,7 @@ pub const ServerWebSocket = struct {
pub fn onPing(this: *ServerWebSocket, _: uws.AnyWebSocket, data: []const u8) void {
log("onPing: {s}", .{data});

var handler = this.handler;
const handler = this.handler;
var cb = handler.onPing;
if (cb.isEmptyOrUndefinedOrNull()) return;
const globalThis = handler.globalObject;
Expand All @@ -4103,21 +4092,22 @@ pub const ServerWebSocket = struct {

if (result.toError()) |err| {
log("onPing error", .{});
handler.globalObject.bunVM().runErrorHandler(err, null);
handler.runErrorCallback(vm, globalThis, err);
}
}

pub fn onPong(this: *ServerWebSocket, _: uws.AnyWebSocket, data: []const u8) void {
log("onPong: {s}", .{data});

var handler = this.handler;
const handler = this.handler;
var cb = handler.onPong;
if (cb.isEmptyOrUndefinedOrNull()) return;

var globalThis = handler.globalObject;
const globalThis = handler.globalObject;
const vm = handler.vm;

// This is the start of a task.
const loop = globalThis.bunVM().eventLoop();
const loop = vm.eventLoop();
loop.enter();
defer loop.exit();

Expand All @@ -4128,7 +4118,7 @@ pub const ServerWebSocket = struct {

if (result.toError()) |err| {
log("onPong error", .{});
handler.globalObject.bunVM().runErrorHandler(err, null);
handler.runErrorCallback(vm, globalThis, err);
}
}

Expand All @@ -4146,7 +4136,8 @@ pub const ServerWebSocket = struct {
if (!handler.onClose.isEmptyOrUndefinedOrNull()) {
var str = ZigString.init(message);
const globalObject = handler.globalObject;
const loop = globalObject.bunVM().eventLoop();
const vm = handler.vm;
const loop = vm.eventLoop();
loop.enter();
defer loop.exit();
str.markUTF8();
Expand All @@ -4157,7 +4148,7 @@ pub const ServerWebSocket = struct {

if (result.toError()) |err| {
log("onClose error", .{});
globalObject.bunVM().runErrorHandler(err, null);
handler.runErrorCallback(vm, globalObject, err);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/bun.js/javascript.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ pub const VirtualMachine = struct {

onUnhandledRejection: *const OnUnhandledRejection = defaultOnUnhandledRejection,
onUnhandledRejectionCtx: ?*anyopaque = null,
onUnhandledRejectionExceptionList: ?*ExceptionList = null,
unhandled_error_counter: usize = 0,
is_handling_uncaught_exception: bool = false,

Expand Down Expand Up @@ -865,7 +866,7 @@ pub const VirtualMachine = struct {
}

pub fn defaultOnUnhandledRejection(this: *JSC.VirtualMachine, _: *JSC.JSGlobalObject, value: JSC.JSValue) void {
this.runErrorHandler(value, null);
this.runErrorHandler(value, this.onUnhandledRejectionExceptionList);
}

pub inline fn packageManager(this: *VirtualMachine) *PackageManager {
Expand Down
3 changes: 2 additions & 1 deletion src/bun.js/test/jest.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ pub const TestRunnerTask = struct {
if (jsc_vm.last_reported_error_for_dedupe == rejection and rejection != .zero) {
jsc_vm.last_reported_error_for_dedupe = .zero;
} else {
jsc_vm.runErrorHandlerWithDedupe(rejection, null);
jsc_vm.runErrorHandlerWithDedupe(rejection, jsc_vm.onUnhandledRejectionExceptionList);
}

if (jsc_vm.onUnhandledRejectionCtx) |ctx| {
Expand Down Expand Up @@ -1321,6 +1321,7 @@ pub const TestRunnerTask = struct {
}

jsc_vm.onUnhandledRejectionCtx = this;
jsc_vm.onUnhandledRejection = onUnhandledRejection;

if (this.needs_before_each) {
this.needs_before_each = false;
Expand Down
16 changes: 15 additions & 1 deletion src/js/thirdparty/ws.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ function emitWarning(type, message) {
console.warn("[bun] Warning:", message);
}

// TODO: add private method on WebSocket to avoid these allocations
function normalizeData(data, opts) {
const isBinary = opts?.binary;
if (isBinary === true && typeof data === "string") {
data = Buffer.from(data);
} else if (isBinary === false && $isTypedArrayView(data)) {
data = new Buffer(data.buffer, data.byteOffset, data.byteLength).toString("utf-8");
}

return data;
}

/**
* @link https://github.com/websockets/ws/blob/master/doc/ws.md#class-websocket
*/
Expand Down Expand Up @@ -129,7 +141,7 @@ class BunWebSocket extends EventEmitter {

send(data, opts, cb) {
try {
this.#ws.send(data, opts?.compress);
this.#ws.send(normalizeData(data, opts), opts?.compress);
} catch (error) {
typeof cb === "function" && cb(error);
return;
Expand Down Expand Up @@ -710,7 +722,9 @@ class BunWebSocketMocked extends EventEmitter {
send(data, opts, cb) {
if (this.#state === 1) {
const compress = opts?.compress;
data = normalizeData(data, opts);
const written = this.#ws.send(data, compress);

if (written == -1) {
// backpressure
this.#enquedMessages.push([data, compress, cb]);
Expand Down
40 changes: 40 additions & 0 deletions test/js/bun/http/bun-serve-propagate-errors.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { spawnSync } from "bun";
import { test, expect } from "bun:test";
import { bunEnv, bunExe, tempDirWithFiles } from "harness";

test("Bun.serve() propagates errors to the parent fixture", async () => {
const code = `import { test } from "bun:test";
test("Bun.serve() propagates errors to the parent", async () => {
const server = Bun.serve({
development: false,
port: 0,
fetch(req) {
throw new Error("Test failed successfully");
},
});
await fetch(server.url);
server.stop(true);
});
`;
const dir = tempDirWithFiles("propagate-errors", {
"package.json": JSON.stringify({
name: "test",
version: "0.0.0",
dependencies: {},
}),
"index.test.ts": code,
});

const { stderr, exitCode } = spawnSync({
cmd: [bunExe(), "test"],
cwd: dir,
env: bunEnv,
stdout: "inherit",
stdin: "inherit",
stderr: "pipe",
});

expect(exitCode).toBe(1);
expect(stderr.toString()).toContain("error: Test failed successfully");
});
11 changes: 7 additions & 4 deletions test/js/bun/websocket/websocket-client-echo.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,23 @@ const ws = new WebSocket(url, {
});

ws.on("open", () => {
console.log("Connected", ws.url); // read by test script
console.error("Connected", ws.url);
if (process.send) {
process.send("connected");
}
console.log(`[${process.versions.bun ? "bun" : "node"}]`, "Connected", ws.url); // read by test script
console.error(`[${process.versions.bun ? "bun" : "node"}]`, "Connected", ws.url);
});

const logMessages = process.env.LOG_MESSAGES === "1";
ws.on("message", (data, isBinary) => {
if (logMessages) {
if (isBinary) {
console.error("Received binary message:", data);
} else {
console.error("Received text message:", data);
data = data.toString();
}
}
ws.send(data, { binary: isBinary });
ws.send(data, { binary: !!isBinary });

if (data === "ping") {
console.error("Sending ping");
Expand Down

0 comments on commit 16920a5

Please sign in to comment.