Skip to content

Commit

Permalink
feat: Add async support to Bun.serve onError (oven-sh#8233)
Browse files Browse the repository at this point in the history
* WIP

* Support not immediately fullfilled promises

* Typo

* Add tests

* Fix test

* Rename test

* Remove file

* [autofix.ci] apply automated fixes

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored and ryoppippi committed Feb 1, 2024
1 parent 2983e6d commit 521abbd
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 1 deletion.
76 changes: 75 additions & 1 deletion src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,8 @@ fn NewFlags(comptime debug_mode: bool) type {
response_protected: bool = false,
aborted: bool = false,
has_finalized: bun.DebugOnly(bool) = bun.DebugOnlyDefault(false),

is_error_promise_pending: bool = false,
};
}

Expand Down Expand Up @@ -1377,7 +1379,7 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
return;
}

if (!resp.hasResponded() and !ctx.flags.has_marked_pending) {
if (!resp.hasResponded() and !ctx.flags.has_marked_pending and !ctx.flags.is_error_promise_pending) {
ctx.renderMissing();
return;
}
Expand Down Expand Up @@ -2951,6 +2953,9 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
if (result.toError()) |err| {
this.finishRunningErrorHandler(err, status);
return;
} else if (result.asAnyPromise()) |promise| {
this.processOnErrorPromise(result, promise, value, status);
return;
} else if (result.as(Response)) |response| {
this.render(response);
return;
Expand All @@ -2961,6 +2966,75 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
this.finishRunningErrorHandler(value, status);
}

fn processOnErrorPromise(
ctx: *RequestContext,
promise_js: JSC.JSValue,
promise: JSC.AnyPromise,
value: JSC.JSValue,
status: u16,
) void {
var vm = ctx.server.vm;

switch (promise.status(vm.global.vm())) {
.Pending => {},
.Fulfilled => {
const fulfilled_value = promise.result(vm.global.vm());

// if you return a Response object or a Promise<Response>
// but you upgraded the connection to a WebSocket
// just ignore the Response object. It doesn't do anything.
// it's better to do that than to throw an error
if (ctx.didUpgradeWebSocket()) {
return;
}

var response = fulfilled_value.as(JSC.WebCore.Response) orelse {
ctx.finishRunningErrorHandler(value, status);
return;
};

ctx.response_jsvalue = fulfilled_value;
ctx.response_jsvalue.ensureStillAlive();
ctx.flags.response_protected = false;
ctx.response_ptr = response;

response.body.value.toBlobIfPossible();
switch (response.body.value) {
.Blob => |*blob| {
if (blob.needsToReadFile()) {
fulfilled_value.protect();
ctx.flags.response_protected = true;
}
},
.Locked => {
fulfilled_value.protect();
ctx.flags.response_protected = true;
},
else => {},
}
ctx.render(response);
return;
},
.Rejected => {
promise.setHandled(vm.global.vm());
ctx.finishRunningErrorHandler(promise.result(vm.global.vm()), status);
return;
},
}

// Promise is not fulfilled yet
{
ctx.flags.is_error_promise_pending = true;
ctx.pending_promises_for_abort += 1;
promise_js.then(
ctx.server.globalThis,
ctx,
RequestContext.onResolve,
RequestContext.onReject,
);
}
}

pub fn runErrorHandlerWithStatusCode(
this: *RequestContext,
value: JSC.JSValue,
Expand Down
53 changes: 53 additions & 0 deletions test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,59 @@ it("should response with HTTP 413 when request body is larger than maxRequestBod

server.stop(true);
});

it("should support promise returned from error", async () => {
const server = Bun.serve({
port: 0,
fetch(req) {
throw new Error(req.url);
},
async error(e) {
if (e.message.endsWith("/async-fulfilled")) {
return new Response("OK");
}

if (e.message.endsWith("/async-rejected")) {
throw new Error("");
}

if (e.message.endsWith("/async-rejected-pending")) {
await Bun.sleep(100);
throw new Error("");
}

if (e.message.endsWith("/async-pending")) {
await Bun.sleep(100);
return new Response("OK");
}
},
});

{
const resp = await fetch(`http://${server.hostname}:${server.port}/async-fulfilled`);
expect(resp.status).toBe(200);
expect(await resp.text()).toBe("OK");
}

{
const resp = await fetch(`http://${server.hostname}:${server.port}/async-pending`);
expect(resp.status).toBe(200);
expect(await resp.text()).toBe("OK");
}

{
const resp = await fetch(`http://${server.hostname}:${server.port}/async-rejected`);
expect(resp.status).toBe(500);
}

{
const resp = await fetch(`http://${server.hostname}:${server.port}/async-rejected-pending`);
expect(resp.status).toBe(500);
}

server.stop(true);
});

if (process.platform === "linux")
it("should use correct error when using a root range port(#7187)", () => {
expect(() => {
Expand Down

0 comments on commit 521abbd

Please sign in to comment.