diff --git a/docs/api/http.md b/docs/api/http.md
index eba77a16c1..17339c5646 100644
--- a/docs/api/http.md
+++ b/docs/api/http.md
@@ -78,7 +78,7 @@ Bun.serve({
fetch(req) {
throw new Error("woops!");
},
- error(error: Error, req: Request | null) {
+ error(error) {
return new Response(`${error}\n${error.stack}`, {
headers: {
"Content-Type": "text/html",
diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts
index d62bd911c2..c8a303fb43 100644
--- a/packages/bun-types/bun.d.ts
+++ b/packages/bun-types/bun.d.ts
@@ -2194,7 +2194,7 @@ declare module "bun" {
*/
development?: boolean;
- error?: (this: Server, error: ErrorLike, request: Request | null) => Response | Promise;
+ error?: (this: Server, request: ErrorLike) => Response | Promise | undefined | Promise;
/**
* Uniquely identify a server instance with an ID
diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig
index a913834768..b2b8e0465f 100644
--- a/src/bun.js/api/server.zig
+++ b/src/bun.js/api/server.zig
@@ -1373,7 +1373,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
if (!has_responded)
ctx.runErrorHandler(
value,
- JSValue.jsNull(),
);
if (ctx.flags.aborted) {
@@ -1999,20 +1998,18 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
file.pathlike.fd
else switch (bun.sys.open(file.pathlike.path.sliceZ(&file_buf), std.os.O.RDONLY | std.os.O.NONBLOCK | std.os.O.CLOEXEC, 0)) {
.result => |_fd| _fd,
- .err => |err| return this.runErrorHandler(
- err.withPath(file.pathlike.path.slice()).toSystemError().toErrorInstance(this.server.globalThis),
- JSValue.jsNull(),
- ),
+ .err => |err| return this.runErrorHandler(err.withPath(file.pathlike.path.slice()).toSystemError().toErrorInstance(
+ this.server.globalThis,
+ )),
};
// stat only blocks if the target is a file descriptor
const stat: bun.Stat = switch (bun.sys.fstat(fd)) {
.result => |result| result,
.err => |err| {
- this.runErrorHandler(
- err.withPathLike(file.pathlike).toSystemError().toErrorInstance(this.server.globalThis),
- JSValue.jsNull(),
- );
+ this.runErrorHandler(err.withPathLike(file.pathlike).toSystemError().toErrorInstance(
+ this.server.globalThis,
+ ));
if (auto_close) {
_ = bun.sys.close(fd);
}
@@ -2032,10 +2029,9 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
};
var sys = err.withPathLike(file.pathlike).toSystemError();
sys.message = bun.String.static("MacOS does not support sending non-regular files");
- this.runErrorHandler(
- sys.toErrorInstance(this.server.globalThis),
- JSValue.jsNull(),
- );
+ this.runErrorHandler(sys.toErrorInstance(
+ this.server.globalThis,
+ ));
return;
}
}
@@ -2052,10 +2048,9 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
};
var sys = err.withPathLike(file.pathlike).toSystemError();
sys.message = bun.String.static("File must be regular or FIFO");
- this.runErrorHandler(
- sys.toErrorInstance(this.server.globalThis),
- JSValue.jsNull(),
- );
+ this.runErrorHandler(sys.toErrorInstance(
+ this.server.globalThis,
+ ));
return;
}
}
@@ -2132,10 +2127,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
}
if (result == .err) {
- this.runErrorHandler(
- result.err.toErrorInstance(this.server.globalThis),
- JSValue.jsNull(),
- );
+ this.runErrorHandler(result.err.toErrorInstance(this.server.globalThis));
return;
}
@@ -2459,7 +2451,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
}
if (response_value.toError()) |err_value| {
- ctx.runErrorHandler(err_value, request_value);
+ ctx.runErrorHandler(err_value);
return;
}
@@ -2674,10 +2666,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
this.finalizeForAbort();
return;
}
- this.runErrorHandler(
- err,
- JSC.JSValue.jsNull(),
- );
+ this.runErrorHandler(err);
return;
},
// .InlineBlob,
@@ -2709,10 +2698,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
.message = bun.String.static("Stream already used, please create a new one"),
};
stream.value.unprotect();
- this.runErrorHandler(
- err.toErrorInstance(this.server.globalThis),
- JSC.JSValue.jsNull(),
- );
+ this.runErrorHandler(err.toErrorInstance(this.server.globalThis));
return;
}
@@ -2896,9 +2882,8 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
pub fn runErrorHandler(
this: *RequestContext,
value: JSC.JSValue,
- request_value: JSValue,
) void {
- runErrorHandlerWithStatusCode(this, value, 500, request_value);
+ runErrorHandlerWithStatusCode(this, value, 500);
}
const PathnameFormatter = struct {
@@ -2959,20 +2944,14 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
this: *RequestContext,
value: JSC.JSValue,
status: u16,
- request_value: JSValue,
) void {
JSC.markBinding(@src());
if (!this.server.config.onError.isEmpty() and !this.flags.has_called_error_handler) {
this.flags.has_called_error_handler = true;
-
- if (Environment.allow_assert) std.debug.assert(request_value != .zero);
const result = this.server.config.onError.callWithThis(
this.server.globalThis,
this.server.thisObject,
- &.{
- value,
- request_value,
- },
+ &.{value},
);
defer result.ensureStillAlive();
if (!result.isEmptyOrUndefinedOrNull()) {
@@ -3065,12 +3044,11 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
this: *RequestContext,
value: JSC.JSValue,
status: u16,
- request_value: JSValue,
) void {
JSC.markBinding(@src());
if (this.resp == null or this.resp.?.hasResponded()) return;
- runErrorHandlerWithStatusCodeDontCheckResponded(this, value, status, request_value);
+ runErrorHandlerWithStatusCodeDontCheckResponded(this, value, status);
}
pub fn renderMetadata(this: *RequestContext) void {
@@ -3366,24 +3344,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
return (this.resp orelse return null).getRemoteSocketInfo();
}
- pub fn toRequestObject(this: *RequestContext, globalThis: *JSGlobalObject) !*JSC.WebCore.Request {
- if (Environment.allow_assert) std.debug.assert(this.signal == null);
- this.signal = JSC.WebCore.AbortSignal.new(globalThis);
-
- if (Environment.allow_assert) std.debug.assert(this.request_body == null);
- this.request_body = JSC.WebCore.InitRequestBodyValue(.{ .Null = {} }) catch unreachable;
-
- const request_object = try this.allocator.create(JSC.WebCore.Request);
- request_object.* = .{
- .method = this.method,
- .request_context = AnyRequestContext.init(this),
- .https = ssl_enabled,
- .signal = this.signal.?.ref(),
- .body = this.request_body.?.ref(),
- };
- return request_object;
- }
-
pub const Export = shim.exportFunctions(.{
.onResolve = onResolve,
.onReject = onReject,
@@ -5899,7 +5859,20 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
var ctx = this.request_pool_allocator.tryGet() catch @panic("ran out of memory");
ctx.create(this, req, resp);
this.vm.jsc.reportExtraMemory(@sizeOf(RequestContext));
- const request_object = ctx.toRequestObject(this.globalThis) catch unreachable;
+ var request_object = this.allocator.create(JSC.WebCore.Request) catch unreachable;
+ var body = JSC.WebCore.InitRequestBodyValue(.{ .Null = {} }) catch unreachable;
+
+ ctx.request_body = body;
+ var signal = JSC.WebCore.AbortSignal.new(this.globalThis);
+ ctx.signal = signal;
+
+ request_object.* = .{
+ .method = ctx.method,
+ .request_context = AnyRequestContext.init(ctx),
+ .https = ssl_enabled,
+ .signal = signal.ref(),
+ .body = body.ref(),
+ };
if (comptime debug_mode) {
ctx.flags.is_web_browser_navigation = brk: {
@@ -5957,8 +5930,10 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
request_object.toJS(this.globalThis),
this.thisObject,
};
+
const request_value = args[0];
request_value.ensureStillAlive();
+
const response_value = this.config.onRequest.callWithThis(this.globalThis, this.thisObject, &args);
defer {
// uWS request will not live longer than this function
@@ -6002,7 +5977,21 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
req.setYield(false);
var ctx = this.request_pool_allocator.tryGet() catch @panic("ran out of memory");
ctx.create(this, req, resp);
- const request_object = ctx.toRequestObject(this.globalThis) catch unreachable;
+ var request_object = this.allocator.create(JSC.WebCore.Request) catch unreachable;
+ var body = JSC.WebCore.InitRequestBodyValue(.{ .Null = {} }) catch unreachable;
+
+ ctx.request_body = body;
+ var signal = JSC.WebCore.AbortSignal.new(this.globalThis);
+ ctx.signal = signal;
+
+ request_object.* = .{
+ .method = ctx.method,
+ .request_context = AnyRequestContext.init(ctx),
+ .upgrader = ctx,
+ .https = ssl_enabled,
+ .signal = signal.ref(),
+ .body = body.ref(),
+ };
ctx.upgrade_context = upgrade_ctx;
// We keep the Request object alive for the duration of the request so that we can remove the pointer to the UWS request object.
diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts
index 452dbb7ac9..dc1aa60ecd 100644
--- a/test/js/bun/http/bun-server.test.ts
+++ b/test/js/bun/http/bun-server.test.ts
@@ -317,7 +317,6 @@ describe("Server", () => {
server.stop(true);
}
});
-
test("abort signal on server with stream", async () => {
{
let signalOnServer = false;
@@ -434,74 +433,6 @@ test("unref keeps process alive for ongoing connections", async () => {
expect([path.join(import.meta.dir, "unref-fixture-2.ts")]).toRun();
});
-describe("Bun.serve error handling", () => {
- test("supports error handling", async () => {
- const server = Bun.serve({
- port: 0,
- fetch(req) {
- throw new Error("woops!");
- },
- error(error) {
- return new Response(`${error.message}`);
- },
- });
-
- const response = await fetch(`http://${server.hostname}:${server.port}`);
- expect(await response.text()).toBe("woops!");
- server.stop(true);
- });
-
- test("supports reading the Request in error handling", async () => {
- const server = Bun.serve({
- port: 0,
- fetch(req) {
- throw new Error("woops!");
- },
- error(error, req) {
- if (req === null) return new Response(`${error.message}`);
- return new Response(`${error.message}\n${req.method}`);
- },
- });
-
- const response = await fetch(`http://${server.hostname}:${server.port}`);
- expect(await response.text()).toBe("woops!\nGET");
- server.stop(true);
- });
-
- test("the request headers survive", async () => {
- const server = Bun.serve({
- port: 0,
- fetch(req) {
- throw new Error("woops!");
- },
- error(error, req) {
- return new Response(`${req.headers.get("x-foo")}`);
- },
- });
-
- const response = await fetch(`http://${server.hostname}:${server.port}`, { headers: { "x-foo": "1" } });
- expect(await response.text()).toBe("1");
- server.stop(true);
- });
-
- test("the request url survives", async () => {
- const server = Bun.serve({
- port: 0,
- fetch(req) {
- throw new Error("woops!");
- },
- error(error, req) {
- return new Response(`${req.url}`);
- },
- });
-
- const url = `http://${server.hostname}:${server.port}/`;
- const response = await fetch(url);
- expect(await response.text()).toBe(url);
- server.stop(true);
- });
-});
-
test("Bun does not crash when given invalid config", async () => {
const server1 = Bun.serve({
fetch(request, server) {