Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub use bun_core::STRING_ALLOCATION_LIMIT;

pub(crate) type OnUnhandledRejection = fn(&mut VirtualMachine, &JSGlobalObject, JSValue);
pub(crate) type MacroMap = bun_collections::ArrayHashMap<i32, JSValue>;
pub type ExceptionList = Vec<crate::exception_list::JsException>;
pub use crate::exception_list::ExceptionList;

// ──────────────────────────────────────────────────────────────────────────
// VirtualMachine struct (file-level @This())
Expand Down Expand Up @@ -6478,12 +6478,8 @@ impl VirtualMachine {
let _ = writer.write_all(b"\n");
}
self.had_errors = self.had_errors || build_error.msg.kind == bun_ast::Kind::Err;
if exception_list.is_some() {
// `log_mut()` is the centralized set-once `Option<NonNull>`
// accessor — single audited deref lives there.
if let Some(log) = self.log_mut() {
let _ = log.add_msg(build_error.msg.clone());
}
if let Some(list) = exception_list {
list.build.push(build_error.msg.clone());
}
bun_core::Output::flush();
return true;
Expand All @@ -6497,12 +6493,8 @@ impl VirtualMachine {
let _ = writer.write_all(b"\n");
}
self.had_errors = self.had_errors || resolve_error.msg.kind == bun_ast::Kind::Err;
if exception_list.is_some() {
// `log_mut()` is the centralized set-once `Option<NonNull>`
// accessor — single audited deref lives there.
if let Some(log) = self.log_mut() {
let _ = log.add_msg(resolve_error.msg.clone());
}
if let Some(list) = exception_list {
list.build.push(resolve_error.msg.clone());
}
bun_core::Output::flush();
return true;
Expand Down Expand Up @@ -6925,6 +6917,15 @@ impl VirtualMachine {
break 'code bun_core::Utf8Bytes::Owned(text.unwrap_or_default());
}
let mut log = bun_ast::Log::default();
// What the resolver logs when the fetch reads the file's directory again is not part of this error.
let mut resolver_log = bun_ast::Log::default();
// SAFETY: the resolver is a field of this VM, and the guard is dropped before `resolver_log`.
let _restore_resolver_log = unsafe {
bun_resolver::Resolver::scoped_log(
&raw mut self.transpiler.resolver,
NonNull::from(&mut resolver_log),
)
};
let Ok(original_source) = bun_removable_features::call_removable_or!(
RuntimeTranspiler,
ReturnDefault,
Expand Down
4 changes: 2 additions & 2 deletions src/jsc/ZigException.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl ZigException<'_> {

pub(crate) fn add_to_error_list(
&mut self,
error_list: &mut Vec<exception_list::JsException>,
error_list: &mut exception_list::ExceptionList,
root_path: &[u8],
origin: Option<&ZigURL>,
) {
Expand All @@ -71,7 +71,7 @@ impl ZigException<'_> {
return;
}

error_list.push(exception_list::JsException {
error_list.exceptions.push(exception_list::JsException {
name: Box::from(name.slice()),
message: Box::from(message.slice()),
runtime_type: self.runtime_type,
Expand Down
10 changes: 9 additions & 1 deletion src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub mod zig_stack_frame_code;
pub mod zig_stack_frame_position;

/// Owned snapshots of a [`ZigException`] (see `ZigException::add_to_error_list`),
/// collected into an [`ExceptionList`](virtual_machine::ExceptionList) so callers
/// collected into an [`ExceptionList`](exception_list::ExceptionList) so callers
/// such as the `Bun.serve` development error page can report errors after the
/// JSC exception itself is gone.
pub mod exception_list {
Expand Down Expand Up @@ -120,6 +120,14 @@ pub mod exception_list {
pub code: JSErrorCode,
pub stack: StackTrace,
}

/// What one error report collected for the caller that asked for it.
#[derive(Default)]
pub struct ExceptionList {
pub exceptions: Vec<JsException>,
/// The message of each `BuildMessage` / `ResolveMessage` in the report.
pub build: Vec<bun_ast::Msg>,
}
}
#[path = "array_buffer.rs"]
pub mod array_buffer;
Expand Down
18 changes: 6 additions & 12 deletions src/runtime/server/DevErrorPage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,19 @@

use std::io::Write as _;

use bun_ast::{Kind, Location, Log, Metadata, Msg};
use bun_ast::{Kind, Location, Metadata, Msg};
use bun_core::fmt::{JSONFormatterUTF8Options, format_json_string_utf8, substitute_named};
use bun_core::strings;
use bun_jsc::exception_list::{JsException, StackTrace};
use bun_jsc::exception_list::{ExceptionList, StackTrace};

const HTML_TEMPLATE: &[u8] = include_bytes!("dev-error-page.html");

pub(crate) struct DevErrorPage<'a> {
/// One-line summary, e.g. `GET /foo failed`.
pub message: &'a [u8],
pub cwd: &'a [u8],
pub exceptions: &'a [JsException],
/// Build/resolve errors logged while handling the request.
pub log: Option<&'a Log>,
/// The thrown errors and the build/resolve errors of the failed request.
pub problems: &'a ExceptionList,
}

impl DevErrorPage<'_> {
Expand Down Expand Up @@ -48,7 +47,7 @@ impl DevErrorPage<'_> {
write_string(w, self.cwd);

w.extend_from_slice(b",\"problems\":{\"exceptions\":[");
for (i, exception) in self.exceptions.iter().enumerate() {
for (i, exception) in self.problems.exceptions.iter().enumerate() {
if i > 0 {
w.push(b',');
}
Expand All @@ -69,12 +68,7 @@ impl DevErrorPage<'_> {
w.push(b'}');
}

// `Log.errors` / `Log.warnings` don't count messages added via
// `add_msg`, which is how build/resolve errors reach this log.
let msgs: &[Msg] = match self.log {
Some(log) => &log.msgs,
None => &[],
};
let msgs = &self.problems.build;
let (mut errors, mut warnings) = (0u32, 0u32);
for msg in msgs {
errors += (msg.kind == Kind::Err) as u32;
Expand Down
22 changes: 6 additions & 16 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1200,12 +1200,7 @@ where
}
}

pub(crate) fn render_default_error(
&self,
log: &bun_ast::Log,
exceptions: &[jsc::exception_list::JsException],
message: &[u8],
) {
pub(crate) fn render_default_error(&self, problems: &jsc::ExceptionList, message: &[u8]) {
if !self.flags.has_written_status() {
self.flags.set_has_written_status(true);
if let Some(resp) = self.resp.get() {
Expand All @@ -1224,8 +1219,7 @@ where
let bb = DevErrorPage {
message,
cwd: bun_resolver::fs::FileSystem::get().top_level_dir(),
exceptions,
log: Some(log),
problems,
}
.render();
let try_end_ok = match self.resp.get() {
Expand Down Expand Up @@ -3040,7 +3034,7 @@ where
return false;
}

let mut exception_list: jsc::ExceptionList = Vec::new();
let mut exception_list = jsc::ExceptionList::default();
server
.vm()
.as_mut()
Expand All @@ -3049,8 +3043,7 @@ where
let bb = DevErrorPage {
message: b"Stream error during server-side rendering",
cwd: bun_resolver::fs::FileSystem::get().top_level_dir(),
exceptions: &exception_list,
log: None,
problems: &exception_list,
}
.render();

Expand Down Expand Up @@ -3677,28 +3670,25 @@ where
// the single audited `&mut VirtualMachine` accessor.
let vm = server.vm().as_mut();
if DEBUG_MODE {
let mut exception_list: jsc::ExceptionList = Vec::new();
let mut exception_list = jsc::ExceptionList::default();
let prev_exception_list = vm.on_unhandled_rejection_exception_list;
vm.on_unhandled_rejection_exception_list = Some(NonNull::from(&mut exception_list));
report_unhandled_error(vm, global_this, value);
vm.on_unhandled_rejection_exception_list = prev_exception_list;

let log = vm.log_mut().unwrap();
bun_core::pretty_errorln!(
"<r><red>{:?}<r> - <b>{}<r> failed",
self.method,
self.ensure_pathname()
);
let msg = format!("{:?} - {} failed", self.method, self.ensure_pathname());
self.render_default_error(log, &exception_list, msg.as_bytes());
log.reset();
self.render_default_error(&exception_list, msg.as_bytes());
return;
}
if status != 404 {
report_unhandled_error(vm, global_this, value);
}
self.render_production_error(status);
vm.log_mut().unwrap().reset();
}

pub(crate) fn run_error_handler_with_status_code_dont_check_responded(
Expand Down
42 changes: 34 additions & 8 deletions test/js/bun/http/serve-stream-body-error-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

83 changes: 83 additions & 0 deletions test/js/bun/http/serve-stream-body-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,89 @@ test.concurrent.each([
expect(stderr).toContain("boom");
});

// The same page for a body that fails with a build or resolve error. Such an
// error has no stack: the page lists it under `problems.build`, where a
// throwing handler's page lists it. The report used to leave this page empty
// and to leave the message in the VM-wide log instead, which printed it a
// second time when the process exited.
const buildMessage = { level: 1, text: "Unexpected ;", on: { build: true, resolve: "" } };
const resolveMessage = {
level: 1,
text: expect.stringMatching(/^Cannot find module '\.\/does-not-exist\.ts'/),
on: { build: false, resolve: "./does-not-exist.ts" },
};
test.concurrent.each([
["mid-stream-build-message", "7\r\nchunk-a\r\n", [buildMessage]],
["mid-stream-resolve-message", "7\r\nchunk-a\r\n", [resolveMessage]],
["mid-stream-both-messages", "7\r\nchunk-a\r\n", [buildMessage, resolveMessage]],
["rewriter-mid-stream-build-message", "e\r\n<b>chunk-a</b>\r\n", [buildMessage]],
])("%s under a dev server: the error page lists the message", async (variant, firstChunk, messages) => {
const { stdout, stderr, exitCode } = await runFixture(variant, "development", "dev-server");
const { body, ...result } = JSON.parse(stdout);
expect({ result, exitCode }).toEqual({
result: {
statusLine: "HTTP/1.1 200 OK",
cleanChunkedTerminator: true,
errorCb: 0,
unhandled: 0,
secondStatusLine: "HTTP/1.1 200 OK",
},
exitCode: 0,
});
expect(body).toStartWith(firstChunk);
const page = JSON.parse(/<script id="__bunfallback" type="application\/json">([^<]*)<\/script>/.exec(body)![1]);
const { build } = page.problems;
expect({
message: page.message,
build: { ...build, msgs: build.msgs.map((msg: any) => ({ level: msg.level, text: msg.data.text, on: msg.on })) },
}).toEqual({
message: "Stream error during server-side rendering",
build: { errors: messages.length, warnings: 0, msgs: messages },
});
// Printed when the body failed, and not again when the process exited.
expect({
build: stderr.split("error: Unexpected ;").length - 1,
resolve: stderr.split("error: Cannot find module './does-not-exist.ts'").length - 1,
}).toEqual({
build: messages.includes(buildMessage) ? 1 : 0,
resolve: messages.includes(resolveMessage) ? 1 : 0,
});
});

// A Worker reports what its VM-wide log holds when it exits, as an `error`
// event at its parent. The messages that the report left in that log made a
// Worker that served the failed body and then finished cleanly fire one.
// Not a variant with a ResolveMessage: a resolution that fails in a Worker leaks a thread-local path buffer.
test.concurrent("mid-stream-build-message under a dev server in a Worker: the parent gets no error event", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const worker = new Worker(${JSON.stringify(fixture)}, {
argv: ["mid-stream-build-message", "development", "dev-server"],
});
const events = [];
worker.addEventListener("error", event => events.push(event.message));
worker.addEventListener("close", event => console.log(JSON.stringify({ events, code: event.code })));`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// One line from the fixture in the Worker, one from the parent.
const lines = stdout
.trim()
.split("\n")
.map(line => JSON.parse(line));
expect(lines.find(line => "body" in line).body).toContain("Stream error during server-side rendering");
expect({ parent: lines.find(line => "events" in line), exitCode }).toEqual({
parent: { events: [], code: 0 },
exitCode: 0,
});
expect(stderr).toContain("error: Unexpected ;");
});

// Natively streamed bodies whose producer failed while nothing was reading them:
// the native stream holds the error, with no JS rejection anywhere, when
// Bun.serve renders the Response. Nothing is committed to the client at that
Expand Down
Loading
Loading