diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index f9d221865a82..8398190b9a1e 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -5,7 +5,7 @@ const EventEmitter = require("node:events"); const { SafeMap } = require("internal/primordials"); const Readable = require("internal/streams/readable"); const Writable = require("internal/streams/writable"); -const { throwNotImplemented, warnNotImplementedOnce } = require("internal/shared"); +const { throwNotImplemented, warnNotImplementedOnce, reportUncaughtException } = require("internal/shared"); const { validateString, validateObject, @@ -1385,7 +1385,18 @@ class Worker extends EventEmitter { (error as any).code = "MODULE_NOT_FOUND"; } } - this.emit("error", error); + if (this.listenerCount("error") > 0) { + this.emit("error", error); + return; + } + // Unhandled: this becomes the parent's uncaught exception. The error came + // from another thread, so there is no throw site here worth showing — + // report the value itself rather than `throw`ing it from inside emit(). + try { + this.emit("error", error); // errorMonitor listeners, ERR_UNHANDLED_ERROR wrapping + } catch (unhandled) { + reportUncaughtException(unhandled); + } } #onMessage(event: MessageEvent) { diff --git a/src/jsc/BuildMessage.rs b/src/jsc/BuildMessage.rs index cd212f69fde1..354848927071 100644 --- a/src/jsc/BuildMessage.rs +++ b/src/jsc/BuildMessage.rs @@ -95,6 +95,25 @@ impl BuildMessage { Ok(build_error.to_js(global)) } + /// The `SyntaxError` this diagnostic becomes when it has to leave the realm + /// it was created in (structured clone / worker error reporting): a + /// `BuildMessage` wraps parser state that can't cross, but its message and + /// file/line/column can. + pub fn to_error_instance(&self, global: &JSGlobalObject) -> JSValue { + let (file, line, column): (&[u8], i32, i32) = match &self.msg.data.location { + Some(loc) => (&loc.file, loc.line, loc.column), + None => (b"", 0, 0), + }; + crate::bun_string_jsc::error_instance_from_location( + global, + crate::JSErrorCode::SyntaxError, + &self.msg.data.text, + file, + line, + column, + ) + } + #[crate::host_fn(method)] pub fn to_string(&self, global: &JSGlobalObject, _frame: &CallFrame) -> JsResult { Ok(self.to_string_fn(global)) @@ -198,3 +217,12 @@ impl BuildMessage { Ok(ZigString::init(self.msg.kind.string()).to_js(global)) } } + +// SerializedScriptValue.cpp +#[unsafe(no_mangle)] +extern "C" fn BuildMessage__toErrorInstance( + this: &BuildMessage, + global: &JSGlobalObject, +) -> JSValue { + this.to_error_instance(global) +} diff --git a/src/jsc/Exception.rs b/src/jsc/Exception.rs index ba0048defec8..939023645ef3 100644 --- a/src/jsc/Exception.rs +++ b/src/jsc/Exception.rs @@ -19,6 +19,8 @@ impl Exception { JSC__Exception__getStackTrace(self, global, stack); } + /// The `JSC::Exception` cell itself as a JSValue (not the thrown value; + /// `JSValue::to_error` unwraps that). pub fn value(&self) -> JSValue { JSC__Exception__asJSValue(self) } diff --git a/src/jsc/JSErrorCode.rs b/src/jsc/JSErrorCode.rs index ae18b10990dd..89bddd8df440 100644 --- a/src/jsc/JSErrorCode.rs +++ b/src/jsc/JSErrorCode.rs @@ -9,7 +9,15 @@ pub struct JSErrorCode(pub u8); #[allow(non_upper_case_globals)] impl JSErrorCode { + // JSC::ErrorType (JavaScriptCore/ErrorType.h) pub const Error: Self = Self(0); + pub const EvalError: Self = Self(1); + pub const RangeError: Self = Self(2); + pub const ReferenceError: Self = Self(3); + pub const SyntaxError: Self = Self(4); + pub const TypeError: Self = Self(5); + pub const URIError: Self = Self(6); + pub const AggregateError: Self = Self(7); // StackOverflow & OutOfMemoryError is not an ErrorType in "JavaScriptCore/ErrorType.h" within JSC, so the number here is just totally made up } diff --git a/src/jsc/ResolveMessage.rs b/src/jsc/ResolveMessage.rs index 594da9ebbbcd..798d618fd731 100644 --- a/src/jsc/ResolveMessage.rs +++ b/src/jsc/ResolveMessage.rs @@ -494,8 +494,36 @@ impl ResolveMessage { }) } + /// See `BuildMessage::to_error_instance`. + pub fn to_error_instance(&self, global: &JSGlobalObject) -> JSValue { + let node_message = self.node_message(); + // Runtime resolution failures know the importing file (`referrer`) but + // carry no `location`; either way the importer is where this points. + let (file, line, column): (&[u8], i32, i32) = match &self.msg.data.location { + Some(loc) => (&loc.file, loc.line, loc.column), + None => (self.referrer.as_deref().unwrap_or(b""), 0, 0), + }; + crate::bun_string_jsc::error_instance_from_location( + global, + crate::JSErrorCode::Error, + node_message.as_deref().unwrap_or(&self.msg.data.text), + file, + line, + column, + ) + } + pub fn finalize(self: Box) { // Dropping the Box drops `msg` and the owned `referrer` buffer. drop(self); } } + +// SerializedScriptValue.cpp +#[unsafe(no_mangle)] +extern "C" fn ResolveMessage__toErrorInstance( + this: &ResolveMessage, + global: &JSGlobalObject, +) -> JSValue { + this.to_error_instance(global) +} diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index fc224cc8f479..f4cf8a33ac6b 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -5490,6 +5490,35 @@ impl VirtualMachine { self.remap_stack_frames_mutex.unlock(); } + fn is_unknown_source(url: &bun_core::String) -> bool { + url.is_empty() + || url.eql_comptime("[unknown]") + // FormatStackTraceForJS spells them this way in `.stack` strings. + || url.eql_comptime("unknown") + || url.eql_comptime("native") + || url.has_prefix_comptime(b"[source:") + } + + /// The frame the source preview describes — the top-most locatable + /// non-runtime frame when hiding internals, else frame 0 — and whether + /// every frame was internal/unlocatable (so frame 0 is only a fallback). + fn preview_frame(&self, frames: &[crate::ZigStackFrame]) -> (usize, bool) { + if !self.hide_bun_stackframes { + return (0, false); + } + for (i, frame) in frames.iter().enumerate() { + if frame.position.is_invalid() + || frame.source_url.has_prefix_comptime(b"bun:") + || frame.source_url.has_prefix_comptime(b"node:") + || Self::is_unknown_source(&frame.source_url) + { + continue; + } + return (i, false); + } + (0, !frames.is_empty()) + } + /// Fills `exception` from `error_instance`, remapping stack frames through source maps. pub(crate) fn remap_zig_exception( &mut self, @@ -5585,9 +5614,7 @@ impl VirtualMachine { fn is_hidden_frame(f: &crate::ZigStackFrame) -> bool { f.source_url.eql_comptime("bun:wrap") || f.function_name.eql_comptime("::bunternal::") } - fn is_unknown_source(url: &bun_core::String) -> bool { - url.is_empty() || url.eql_comptime("[unknown]") || url.has_prefix_comptime(b"[source:") - } + let is_unknown_source = Self::is_unknown_source; let mut frames_len = exception.stack.frames_len as usize; // SAFETY: `frames_ptr[..frames_len]` is the caller-owned `Holder` @@ -5621,13 +5648,14 @@ impl VirtualMachine { { continue; } - // Note: `frames[j] = frame`. `ZigStackFrame` impls - // `Drop` so `copy_within` is unavailable; swap instead — - // the discarded tail past `j` is never read after we - // truncate `frames_len` below. + // Swap rather than copy so each frame's refs stay owned + // exactly once; the discarded frames collect past `j`. frames_buf.swap(j, i); j += 1; } + for discarded in &mut frames_buf[j..frames_len] { + discarded.deinit(); + } exception.stack.frames_len = j as u8; frames_len = j; } @@ -5638,27 +5666,7 @@ impl VirtualMachine { return; } - // Pick the top-most non-builtin frame for source preview. - let mut top: usize = 0; - let mut top_frame_is_builtin = false; - if self.hide_bun_stackframes { - for (i, frame) in frames.iter().enumerate() { - if frame.source_url.has_prefix_comptime(b"bun:") - || frame.source_url.has_prefix_comptime(b"node:") - || frame.source_url.is_empty() - || frame.source_url.eql_comptime("native") - || frame.source_url.eql_comptime("unknown") - || frame.source_url.eql_comptime("[unknown]") - || frame.source_url.has_prefix_comptime(b"[source:") - { - top_frame_is_builtin = true; - continue; - } - top = i; - top_frame_is_builtin = false; - break; - } - } + let (top, top_frame_is_builtin) = self.preview_frame(frames); // Don't show source code preview for REPL frames — it would show the // transformed IIFE wrapper code, not what the user typed. @@ -5669,32 +5677,37 @@ impl VirtualMachine { let top_source_url = frames[top].source_url.to_utf8(); let already_remapped = frames[top].remapped; - let maybe_lookup: Option = if already_remapped { - Some(bun_sourcemap::mapping::Lookup { - mapping: bun_sourcemap::mapping::Mapping { - generated: bun_sourcemap::LineColumnOffset::default(), - original: bun_sourcemap::LineColumnOffset { - lines: bun_sourcemap::Ordinal::from_zero_based( - frames[top].position.line.zero_based().max(0), - ), - columns: bun_sourcemap::Ordinal::from_zero_based( - frames[top].position.column.zero_based().max(0), - ), + let maybe_lookup: Option = + if already_remapped && !frames[top].position.line.is_valid() { + // `at ` with no line (e.g. parsed from a `.stack`): nothing + // to look up or preview. + None + } else if already_remapped { + Some(bun_sourcemap::mapping::Lookup { + mapping: bun_sourcemap::mapping::Mapping { + generated: bun_sourcemap::LineColumnOffset::default(), + original: bun_sourcemap::LineColumnOffset { + lines: bun_sourcemap::Ordinal::from_zero_based( + frames[top].position.line.zero_based().max(0), + ), + columns: bun_sourcemap::Ordinal::from_zero_based( + frames[top].position.column.zero_based().max(0), + ), + }, + source_index: 0, + name_index: -1, }, - source_index: 0, - name_index: -1, - }, - source_map: None, - prefetched_source_code: None, - }) - } else { - self.resolve_source_mapping( - top_source_url.slice(), - frames[top].position.line, - frames[top].position.column, - bun_sourcemap::SourceContentHandling::SourceContents, - ) - }; + source_map: None, + prefetched_source_code: None, + }) + } else { + self.resolve_source_mapping( + top_source_url.slice(), + frames[top].position.line, + frames[top].position.column, + bun_sourcemap::SourceContentHandling::SourceContents, + ) + }; if let Some(lookup) = maybe_lookup { // The source-map Arc drops on scope exit. @@ -5767,12 +5780,14 @@ impl VirtualMachine { }; if enable_source_code_preview.get() && code.slice().is_empty() { - exception.collect_source_lines(error_instance, global); + exception.collect_source_lines(top); } - // Direct copy; both sides are `bun_core::Ordinal`. - frames[top].position.line = mapping.original.lines; - frames[top].position.column = mapping.original.columns; + if !already_remapped { + // Direct copy; both sides are `bun_core::Ordinal`. + frames[top].position.line = mapping.original.lines; + frames[top].position.column = mapping.original.columns; + } exception.remapped = true; frames[top].remapped = true; @@ -5811,14 +5826,14 @@ impl VirtualMachine { *source_code_slice = Some(code); } } else if enable_source_code_preview.get() { - exception.collect_source_lines(error_instance, global); + exception.collect_source_lines(top); } drop(top_source_url); if frames.len() > 1 { for i in 0..frames.len() { - if i == top || frames[i].position.is_invalid() { + if i == top || frames[i].position.is_invalid() || frames[i].remapped { continue; } let source_url = frames[i].source_url.to_utf8(); @@ -6163,19 +6178,8 @@ impl VirtualMachine { } let frames = exception.stack.frames(); - let mut top_frame: Option<&crate::ZigStackFrame> = frames.first(); - if self.hide_bun_stackframes { - for frame in frames { - if frame.position.is_invalid() - || frame.source_url.has_prefix_comptime(b"bun:") - || frame.source_url.has_prefix_comptime(b"node:") - { - continue; - } - top_frame = Some(frame); - break; - } - } + let top_frame: Option<&crate::ZigStackFrame> = + frames.get(self.preview_frame(frames).0); let trimmed = source.trimmed_text(); diff --git a/src/jsc/ZigException.rs b/src/jsc/ZigException.rs index 7852d6f5d457..3b3b866f576f 100644 --- a/src/jsc/ZigException.rs +++ b/src/jsc/ZigException.rs @@ -8,16 +8,14 @@ use bun_url::URL as ZigURL; use crate::module_loader::ModuleLoader; use crate::virtual_machine::VirtualMachine; -use crate::{JSErrorCode, JSGlobalObject, JSRuntimeType, JSValue, ZigStackFrame, ZigStackTrace}; +use crate::{JSErrorCode, JSRuntimeType, ZigStackFrame, ZigStackTrace}; -// SAFETY (safe fn): `JSValue` is a by-value scalar; `JSGlobalObject` is an -// opaque `UnsafeCell`-backed handle (`&` is ABI-identical to non-null `*mut`); -// `ZigException` is a `#[repr(C)]` out-param the C++ side fills in-place. +// SAFETY (safe fn): `ZigException` is a `#[repr(C)]` out-param the C++ side +// fills in-place. unsafe extern "C" { pub(crate) safe fn ZigException__collectSourceLines( - js_value: JSValue, - global: &JSGlobalObject, exception: &mut ZigException, + top_frame: u8, ); } @@ -50,8 +48,10 @@ pub struct ZigException { } impl ZigException { - pub(crate) fn collect_source_lines(&mut self, value: JSValue, global: &JSGlobalObject) { - ZigException__collectSourceLines(value, global, self); + /// Slice frame `top_frame`'s source lines out of the SourceProvider it was + /// populated from (for sources the printer can't read from disk). + pub(crate) fn collect_source_lines(&mut self, top_frame: usize) { + ZigException__collectSourceLines(self, u8::try_from(top_frame).unwrap_or(u8::MAX)); } // Kept as explicit `deinit` (not `Drop`) — this is a #[repr(C)] FFI diff --git a/src/jsc/ZigStackFrame.rs b/src/jsc/ZigStackFrame.rs index 4de668b7810d..2ae77c2dbe6a 100644 --- a/src/jsc/ZigStackFrame.rs +++ b/src/jsc/ZigStackFrame.rs @@ -1,4 +1,5 @@ use core::fmt; +use core::ptr::NonNull; use std::io::Write as _; use bstr::BStr; @@ -23,8 +24,10 @@ pub struct ZigStackFrame { /// This informs formatters whether to display as a blob URL or not pub remapped: bool, - /// -1 means not set. - pub jsc_stack_frame_index: i32, + /// Ref'd by C++ when `position` was computed from a JSC frame: the + /// `SourceProvider` that position indexes into, so + /// `ZigException::collect_source_lines` can slice the preview out of it. + pub source_provider: Option>, } impl ZigStackFrame { @@ -40,6 +43,9 @@ impl ZigStackFrame { pub(crate) fn deinit(&mut self) { self.function_name.deref(); self.source_url.deref(); + if let Some(provider) = self.source_provider.take() { + crate::SourceProvider::opaque_mut(provider.as_ptr()).deref(); + } } pub(crate) fn snapshot( @@ -72,7 +78,7 @@ impl ZigStackFrame { position: ZigStackFramePosition::INVALID, is_async: false, remapped: false, - jsc_stack_frame_index: -1, + source_provider: None, }; pub fn name_formatter(&self, enable_color: bool) -> NameFormatter { @@ -173,7 +179,7 @@ impl<'a> fmt::Display for SourceURLFormatter<'a> { if self.line_column == LineColumn::Include && !source_slice.is_empty() - && (self.position.line.is_valid() || self.position.column.is_valid()) + && self.position.line.is_valid() { if self.enable_color { f.write_str(Output::pretty_fmt!(":", true))?; diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index ea84d276eda9..b0335489fd0a 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -56,11 +56,6 @@ static WTF::StringView StringView_slice(WTF::StringView sv, unsigned start, unsi using namespace JSC; using namespace WebCore; -enum PopulateStackTraceFlags { - OnlyPosition, - OnlySourceLines, -}; - #define SYNTAX_ERROR_CODE 4 using Zig::FinalizerSafety; @@ -127,9 +122,7 @@ static void populateStackFrameMetadata(JSC::VM& vm, JSC::JSGlobalObject* globalO frame.is_async = stackFrame.isAsyncFrame(); } -static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunString* source_lines, - OrdinalNumber* source_line_numbers, uint8_t source_lines_count, - ZigStackFramePosition& position, JSC::SourceProvider** referenced_source_provider, PopulateStackTraceFlags flags) +static void populateStackFramePosition(const JSC::StackFrame& stackFrame, ZigStackFrame& frame) { auto code = stackFrame.codeBlock(); if (!code) @@ -147,95 +140,81 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr if (!stackFrame.hasBytecodeIndex()) { if (stackFrame.hasLineAndColumnInfo()) { auto lineColumn = stackFrame.computeLineAndColumn(); - position.line_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt(); - position.column_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt(); + frame.position.line_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt(); + frame.position.column_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt(); } - position.byte_position = -1; + frame.position.byte_position = -1; return; } auto location = Bun::getAdjustedPositionForBytecode(code, stackFrame.bytecodeIndex()); - memcpy(&position, &location, sizeof(ZigStackFramePosition)); - if (flags == PopulateStackTraceFlags::OnlyPosition) - return; - - if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) { - // Search for the beginning of the line - unsigned int lineStart = location.byte_position; - while (lineStart > 0 && sourceString[lineStart] != '\n') { - lineStart--; - } - - // Search for the end of the line - unsigned int lineEnd = location.byte_position; - unsigned int maxSearch = sourceString.length(); - while (lineEnd < maxSearch && sourceString[lineEnd] != '\n') { - lineEnd++; - } - - const unsigned char* bytes = sourceString.span8().data(); - - // Most of the time, when you look at a stack trace, you want a couple lines above. - - // It is key to not clone this data because source code strings are large. - // Usage of toStringView (non-owning) is safe as we ref the provider. - provider->ref(); - if (*referenced_source_provider != nullptr) { - (*referenced_source_provider)->deref(); - } - *referenced_source_provider = provider; - source_lines[0] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart)); - source_line_numbers[0] = location.line(); - - if (lineStart > 0) { - auto byte_offset_in_source_string = lineStart - 1; - uint8_t source_line_i = 1; - auto remaining_lines_to_grab = source_lines_count - 1; - - { - // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { - byte_offset_in_source_string--; - } - - byte_offset_in_source_string -= byte_offset_in_source_string > 0; - } - - while (byte_offset_in_source_string > 0 && remaining_lines_to_grab > 0) { - unsigned int end_of_line_offset = byte_offset_in_source_string; - - // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { - byte_offset_in_source_string--; - } - - // We are at the beginning of the line - source_lines[source_line_i] = Bun::toStringView(sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1)); - - source_line_numbers[source_line_i] = location.line().fromZeroBasedInt(location.line().zeroBasedInt() - source_line_i); - source_line_i++; - - remaining_lines_to_grab--; + memcpy(&frame.position, &location, sizeof(ZigStackFramePosition)); + + // Pin the provider so the frame's source lines can be sliced out later + // (collectSourceLines) without going back to JSC's weakly-held frames. + provider->ref(); + if (frame.source_provider) + frame.source_provider->deref(); + frame.source_provider = provider; +} - byte_offset_in_source_string -= byte_offset_in_source_string > 0; - } - } - } +static void populateStackFrame(JSC::VM& vm, const JSC::StackFrame& stackFrame, ZigStackFrame& frame, JSC::JSGlobalObject* globalObject, FinalizerSafety finalizerSafety) +{ + populateStackFrameMetadata(vm, globalObject, stackFrame, frame, finalizerSafety); + populateStackFramePosition(stackFrame, frame); } -static void populateStackFrame(JSC::VM& vm, ZigStackTrace& trace, const JSC::StackFrame& stackFrame, - ZigStackFrame& frame, bool is_top, JSC::SourceProvider** referenced_source_provider, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety) +// Frame `topFrame`'s line and a few above it, for the source preview. Most of +// the time the printer reads the original file itself; this is the fallback +// for sources that only exist in memory (eval, builtins, blobs). +static void collectSourceLines(ZigStackTrace& trace, uint8_t topFrame) { - if (flags == PopulateStackTraceFlags::OnlyPosition) { - populateStackFrameMetadata(vm, globalObject, stackFrame, frame, finalizerSafety); - populateStackFramePosition(stackFrame, nullptr, - nullptr, - 0, frame.position, referenced_source_provider, flags); - } else if (flags == PopulateStackTraceFlags::OnlySourceLines) { - populateStackFramePosition(stackFrame, is_top ? trace.source_lines_ptr : nullptr, - is_top ? trace.source_lines_numbers : nullptr, - is_top ? trace.source_lines_to_collect : 0, frame.position, referenced_source_provider, flags); + if (topFrame >= trace.frames_len || trace.source_lines_ptr == nullptr || trace.source_lines_to_collect <= 1) + return; + ZigStackFrame& top = trace.frames_ptr[topFrame]; + JSC::SourceProvider* provider = top.source_provider; + if (!provider || top.position.byte_position < 0) + return; + WTF::StringView sourceString = provider->source(); + if (sourceString.isNull() || !sourceString.is8Bit() || static_cast(top.position.byte_position) >= sourceString.length()) + return; + + BunString* source_lines = trace.source_lines_ptr; + OrdinalNumber* source_line_numbers = trace.source_lines_numbers; + const uint8_t source_lines_count = trace.source_lines_to_collect; + const OrdinalNumber line = top.position.line(); + const unsigned length = sourceString.length(); + const Latin1Character* bytes = sourceString.span8().data(); + + // It is key to not clone this data because source code strings are large. + // Usage of toStringView (non-owning) is safe as we ref the provider. + provider->ref(); + if (trace.referenced_source_provider != nullptr) { + trace.referenced_source_provider->deref(); + } + trace.referenced_source_provider = provider; + + // The top frame's line… + unsigned lineStart = top.position.byte_position; + while (lineStart > 0 && bytes[lineStart - 1] != '\n') + lineStart--; + unsigned lineEnd = top.position.byte_position; + while (lineEnd < length && bytes[lineEnd] != '\n') + lineEnd++; + source_lines[0] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart)); + source_line_numbers[0] = line; + + // …and a few above it, since that is what you want to see in a stack trace. + uint8_t i = 1; + while (i < source_lines_count && lineStart > 0) { + lineEnd = lineStart - 1; // the '\n' ending the line above + lineStart = lineEnd; + while (lineStart > 0 && bytes[lineStart - 1] != '\n') + lineStart--; + source_lines[i] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart)); + source_line_numbers[i] = OrdinalNumber::fromZeroBasedInt(line.zeroBasedInt() - i); + i++; } } @@ -245,8 +224,8 @@ class V8StackTraceIterator { public: StringView functionName {}; StringView sourceURL {}; - WTF::OrdinalNumber lineNumber = WTF::OrdinalNumber::fromZeroBasedInt(0); - WTF::OrdinalNumber columnNumber = WTF::OrdinalNumber::fromZeroBasedInt(0); + WTF::OrdinalNumber lineNumber = WTF::OrdinalNumber::beforeFirst(); + WTF::OrdinalNumber columnNumber = WTF::OrdinalNumber::beforeFirst(); bool isConstructor = false; bool isGlobalCode = false; @@ -296,22 +275,22 @@ class V8StackTraceIterator { if (openingParentheses > closingParentheses) openingParentheses = WTF::notFound; + StringView functionName; + StringView lineInner; if (openingParentheses == WTF::notFound || closingParentheses == WTF::notFound) { - // Special case: "unknown" frames don't have parentheses but are valid - // These appear in stack traces from certain error paths - if (line == "unknown"_s) { - frame.sourceURL = line; - frame.functionName = StringView(); - return true; + // `at ::` — anonymous and top-level frames carry + // no name and no parentheses (V8 and Bun both print them this way). + lineInner = line; + if (lineInner.startsWith("async "_s)) { + frame.isAsync = true; + lineInner = lineInner.substring(6); } - - // For any other frame without parentheses, terminate parsing as before - offset = stack.length(); - return false; + } else { + lineInner = StringView_slice(line, openingParentheses + 1, closingParentheses); + if (openingParentheses > 0) + functionName = line.substring(0, openingParentheses - 1); } - auto lineInner = StringView_slice(line, openingParentheses + 1, closingParentheses); - { auto marker1 = 0; auto marker2 = lineInner.find(':', marker1); @@ -383,8 +362,6 @@ class V8StackTraceIterator { } done_block: - StringView functionName = line.substring(0, openingParentheses - 1); - if (functionName == "global code"_s) { functionName = StringView(); frame.isGlobalCode = true; @@ -400,10 +377,6 @@ class V8StackTraceIterator { functionName = functionName.substring(4); } - if (functionName == ""_s) { - functionName = StringView(); - } - frame.functionName = functionName; return true; @@ -421,37 +394,26 @@ class V8StackTraceIterator { } }; -static void populateStackTrace(JSC::VM& vm, const WTF::Vector& frames, ZigStackTrace& trace, JSC::JSGlobalObject* globalObject, PopulateStackTraceFlags flags, FinalizerSafety finalizerSafety = FinalizerSafety::NotInFinalizer) +static void populateStackTrace(JSC::VM& vm, const WTF::Vector& frames, ZigStackTrace& trace, JSC::JSGlobalObject* globalObject, FinalizerSafety finalizerSafety = FinalizerSafety::NotInFinalizer) { - if (flags == PopulateStackTraceFlags::OnlyPosition) { - uint8_t frame_i = 0; - size_t stack_frame_i = 0; - const size_t total_frame_count = frames.size(); - const uint8_t frame_count = total_frame_count < trace.frames_cap ? total_frame_count : trace.frames_cap; - - while (frame_i < frame_count && stack_frame_i < total_frame_count) { - // Skip native frames - while (stack_frame_i < total_frame_count && !(frames.at(stack_frame_i).hasLineAndColumnInfo()) && !(frames.at(stack_frame_i).isWasmFrame())) { - stack_frame_i++; - } - if (stack_frame_i >= total_frame_count) - break; - - ZigStackFrame& frame = trace.frames_ptr[frame_i]; - frame.jsc_stack_frame_index = static_cast(stack_frame_i); - populateStackFrame(vm, trace, frames[stack_frame_i], frame, frame_i == 0, &trace.referenced_source_provider, globalObject, flags, finalizerSafety); + uint8_t frame_i = 0; + size_t stack_frame_i = 0; + const size_t total_frame_count = frames.size(); + const uint8_t frame_count = total_frame_count < trace.frames_cap ? total_frame_count : trace.frames_cap; + + while (frame_i < frame_count && stack_frame_i < total_frame_count) { + // Skip native frames + while (stack_frame_i < total_frame_count && !(frames.at(stack_frame_i).hasLineAndColumnInfo()) && !(frames.at(stack_frame_i).isWasmFrame())) { stack_frame_i++; - frame_i++; - } - trace.frames_len = frame_i; - } else if (flags == PopulateStackTraceFlags::OnlySourceLines) { - for (uint8_t i = 0; i < trace.frames_len; i++) { - ZigStackFrame& frame = trace.frames_ptr[i]; - if (frame.jsc_stack_frame_index < 0 || static_cast(frame.jsc_stack_frame_index) >= frames.size()) - continue; - populateStackFrame(vm, trace, frames[frame.jsc_stack_frame_index], frame, i == 0, &trace.referenced_source_provider, globalObject, flags, finalizerSafety); } + if (stack_frame_i >= total_frame_count) + break; + + populateStackFrame(vm, frames[stack_frame_i], trace.frames_ptr[frame_i], globalObject, finalizerSafety); + stack_frame_i++; + frame_i++; } + trace.frames_len = frame_i; } static JSC::JSValue getNonObservable(JSC::VM& vm, JSC::JSGlobalObject* global, JSC::JSObject* obj, const JSC::PropertyName& propertyName) @@ -473,21 +435,16 @@ static JSC::JSValue getNonObservable(JSC::VM& vm, JSC::JSGlobalObject* global, J static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, JSC::ErrorInstance* err, const Vector* stackTrace, - JSC::JSValue val, PopulateStackTraceFlags flags) + JSC::JSValue val) { JSC::JSObject* obj = dynamicDowncast(val); auto& vm = JSC::getVM(global); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); - bool getFromSourceURL = false; if (stackTrace != nullptr && stackTrace->size() > 0) { - populateStackTrace(vm, *stackTrace, except.stack, global, flags); - + populateStackTrace(vm, *stackTrace, except.stack, global); } else if (err->stackTrace() != nullptr && err->stackTrace()->size() > 0) { - populateStackTrace(vm, *err->stackTrace(), except.stack, global, flags, FinalizerSafety::MustNotTriggerGC); - - } else { - getFromSourceURL = true; + populateStackTrace(vm, *err->stackTrace(), except.stack, global, FinalizerSafety::MustNotTriggerGC); } except.type = (unsigned char)err->errorType(); if (err->isStackOverflowError()) { @@ -574,8 +531,7 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, } } - if (getFromSourceURL) { - + if (except.stack.frames_len == 0) { // we don't want to serialize JSC::StackFrame longer than we need to // so in this case, we parse the stack trace as a string @@ -602,10 +558,17 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, iterator.forEachFrame([&](const V8StackTraceIterator::StackFrame& frame, bool& stop) -> void { ASSERT(except.stack.frames_len < frame_count); + // A frame that can't be located isn't worth a slot: no URL, the + // `unknown`/`native` placeholders populateStackTrace would have skipped + // as native frames, or a bare word with neither line nor path. + bool hasLine = frame.lineNumber.zeroBasedInt() >= 0; + bool urlIsPath = frame.sourceURL.find('/') != WTF::notFound || frame.sourceURL.find('\\') != WTF::notFound; + if (frame.sourceURL.isEmpty() || (!hasLine && (frame.sourceURL == "unknown"_s || frame.sourceURL == "native"_s || (frame.functionName.isEmpty() && !urlIsPath)))) + return; auto& current = except.stack.frames_ptr[except.stack.frames_len]; current = {}; - String functionName = frame.functionName.toString(); + String functionName = frame.functionName == ""_s ? String() : frame.functionName.toString(); String sourceURL = frame.sourceURL.toString(); current.function_name = Bun::toStringRef(functionName); current.source_url = Bun::toStringRef(sourceURL); @@ -619,6 +582,8 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, current.code_type = ZigStackFrameCodeConstructor; } else if (frame.isGlobalCode) { current.code_type = ZigStackFrameCodeGlobal; + } else if (!frame.functionName.isEmpty()) { + current.code_type = ZigStackFrameCodeFunction; } except.stack.frames_len += 1; @@ -626,21 +591,19 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, stop = except.stack.frames_len >= frame_count; }); - if (except.stack.frames_len > 0) { - getFromSourceURL = false; + if (except.stack.frames_len > 0) except.remapped = true; - } } } } } - if (except.stack.frames_len == 0 && getFromSourceURL) { + if (except.stack.frames_len == 0) { JSC::JSValue sourceURL = getNonObservable(vm, global, obj, vm.propertyNames->sourceURL); if (!scope.clearExceptionExceptTermination()) [[unlikely]] return; if (sourceURL) { - if (sourceURL.isString()) { + if (sourceURL.isString() && asString(sourceURL)->length()) { except.stack.frames_ptr[0].source_url.deref(); except.stack.frames_ptr[0].source_url = Bun::toStringRef(global, sourceURL); if (!scope.clearExceptionExceptTermination()) [[unlikely]] @@ -680,14 +643,7 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, } } } - } - { - for (int i = 1; i < except.stack.frames_len; i++) { - auto frame = except.stack.frames_ptr[i]; - frame.function_name.deref(); - frame.source_url.deref(); - } except.stack.frames_len = 1; PropertySlot slot = PropertySlot(obj, PropertySlot::InternalMethodType::VMInquiry, &vm); except.stack.frames_ptr[0].remapped = obj->getNonIndexPropertySlot(global, names.originalLinePublicName(), slot); @@ -828,7 +784,7 @@ void exceptionFromString(ZigException& except, JSC::JSValue value, JSC::JSGlobal extern "C" void JSC__Exception__getStackTrace(JSC::Exception* arg0, JSC::JSGlobalObject* global, ZigStackTrace* trace) { - populateStackTrace(arg0->vm(), arg0->stack(), *trace, global, PopulateStackTraceFlags::OnlyPosition); + populateStackTrace(arg0->vm(), arg0->stack(), *trace, global); } extern "C" [[ZIG_EXPORT(check_slow)]] void JSC__JSValue__toZigException(JSC::EncodedJSValue jsException, JSC::JSGlobalObject* global, ZigException* exception) @@ -846,12 +802,12 @@ extern "C" [[ZIG_EXPORT(check_slow)]] void JSC__JSValue__toZigException(JSC::Enc JSValue unwrapped = jscException->value(); if (JSC::ErrorInstance* error = dynamicDowncast(unwrapped)) { - fromErrorInstance(*exception, global, error, &jscException->stack(), unwrapped, PopulateStackTraceFlags::OnlyPosition); + fromErrorInstance(*exception, global, error, &jscException->stack(), unwrapped); return; } if (jscException->stack().size() > 0) { - populateStackTrace(global->vm(), jscException->stack(), exception->stack, global, PopulateStackTraceFlags::OnlyPosition); + populateStackTrace(global->vm(), jscException->stack(), exception->stack, global); } exceptionFromString(*exception, unwrapped, global); @@ -859,36 +815,14 @@ extern "C" [[ZIG_EXPORT(check_slow)]] void JSC__JSValue__toZigException(JSC::Enc } if (JSC::ErrorInstance* error = dynamicDowncast(value)) { - fromErrorInstance(*exception, global, error, nullptr, value, PopulateStackTraceFlags::OnlyPosition); + fromErrorInstance(*exception, global, error, nullptr, value); return; } exceptionFromString(*exception, value, global); } -extern "C" void ZigException__collectSourceLines(JSC::EncodedJSValue jsException, JSC::JSGlobalObject* global, ZigException* exception) +extern "C" void ZigException__collectSourceLines(ZigException* exception, uint8_t topFrame) { - JSC::JSValue value = JSC::JSValue::decode(jsException); - if (value == JSC::JSValue {}) { - return; - } - - if (value.classInfoOrNull() == JSC::Exception::info()) { - auto* jscException = uncheckedDowncast(value); - JSValue unwrapped = jscException->value(); - - if (jscException->stack().size() > 0) { - populateStackTrace(global->vm(), jscException->stack(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines); - } - - exceptionFromString(*exception, unwrapped, global); - return; - } - - if (JSC::ErrorInstance* error = dynamicDowncast(value)) { - if (error->stackTrace() != nullptr && error->stackTrace()->size() > 0) { - populateStackTrace(global->vm(), *error->stackTrace(), exception->stack, global, PopulateStackTraceFlags::OnlySourceLines, FinalizerSafety::MustNotTriggerGC); - } - return; - } + collectSourceLines(exception->stack, topFrame); } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index b024bc42ca6c..5aa40b5c49ac 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -6528,6 +6528,17 @@ extern "C" EncodedJSValue JSC__createRangeError(JSC::JSGlobalObject* globalObjec return JSValue::encode(JSC::createRangeError(globalObject, str->toWTFString(BunString::ZeroCopy))); } +// An ErrorInstance whose location is supplied rather than captured from the +// current JS stack — the same constructor structured-clone deserialization +// uses. `line`/`column`/`sourceURL`/`stack` materialize lazily as the usual +// non-enumerable own properties. +extern "C" EncodedJSValue JSC__createErrorWithLocation(JSC::JSGlobalObject* globalObject, uint8_t errorType, const BunString* message, const BunString* sourceURL, int32_t line, int32_t column, const BunString* stack) +{ + return JSValue::encode(JSC::ErrorInstance::create(globalObject, message->toWTFString(), static_cast(errorType), + { line < 0 ? 0u : static_cast(line), column < 0 ? 0u : static_cast(column) }, + sourceURL->toWTFString(), stack->toWTFString())); +} + extern "C" EncodedJSValue ExpectMatcherUtils__getSingleton(JSC::JSGlobalObject* globalObject_) { Zig::GlobalObject* globalObject = static_cast(globalObject_); diff --git a/src/jsc/bindings/headers-handwritten.h b/src/jsc/bindings/headers-handwritten.h index 837f9615141b..557f48ea7743 100644 --- a/src/jsc/bindings/headers-handwritten.h +++ b/src/jsc/bindings/headers-handwritten.h @@ -199,7 +199,9 @@ typedef struct ZigStackFrame { ZigStackFrameCode code_type; bool is_async; bool remapped; - int32_t jsc_stack_frame_index; + // Ref'd; the SourceProvider `position` was computed against, so source + // lines can be sliced from it later. + JSC::SourceProvider* source_provider; ZigStackFrame() : function_name {} @@ -208,7 +210,7 @@ typedef struct ZigStackFrame { , code_type {} , is_async(false) , remapped(false) - , jsc_stack_frame_index(-1) + , source_provider(nullptr) { } } ZigStackFrame; diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index a6ada2d00b66..851ed7ba9e27 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -205,6 +205,27 @@ enum SerializationTag { // pointer is gone. extern "C" SYSV_ABI void BlockList__onStructuredCloneDestroy(void*); +extern "C" JSC::EncodedJSValue BuildMessage__toErrorInstance(void*, JSC::JSGlobalObject*); +extern "C" JSC::EncodedJSValue ResolveMessage__toErrorInstance(void*, JSC::JSGlobalObject*); + +// Bun's parse and resolve diagnostics have Error.prototype in their chain but +// wrap VM-local parser state, so they serialize as the plain SyntaxError / +// Error carrying their message and location. +static bool serializesAsError(JSC::JSObject* object) +{ + return object->isErrorInstance() || object->inherits() || object->inherits(); +} + +// The ErrorInstance to write for an `object` that `serializesAsError`. +static JSC::ErrorInstance* toSerializableErrorInstance(JSC::JSGlobalObject* globalObject, JSC::JSObject* object) +{ + if (object->isErrorInstance()) + return uncheckedDowncast(object); + if (auto* buildMessage = dynamicDowncast(object)) + return uncheckedDowncast(JSC::JSValue::decode(BuildMessage__toErrorInstance(buildMessage->wrapped(), globalObject))); + return uncheckedDowncast(JSC::JSValue::decode(ResolveMessage__toErrorInstance(uncheckedDowncast(object)->wrapped(), globalObject))); +} + enum ArrayBufferViewSubtag { DataViewTag = 0, Int8ArrayTag = 1, @@ -1183,9 +1204,10 @@ class CloneSerializer : public CloneBase { write(String::fromLatin1(JSC::Yarr::flagsString(regExp->regExp()->flags()).data())); return true; } - if (auto* errorInstance = dynamicDowncast(obj)) { - if (!startObjectInternal(errorInstance)) // handle duplicates + if (serializesAsError(obj)) { + if (!startObjectInternal(obj)) // handle duplicates return true; + auto* errorInstance = toSerializableErrorInstance(m_lexicalGlobalObject, obj); auto& vm = m_lexicalGlobalObject->vm(); auto errorTypeValue = errorInstance->get(m_lexicalGlobalObject, vm.propertyNames->name); RETURN_IF_EXCEPTION(scope, false); diff --git a/src/jsc/bun_string_jsc.rs b/src/jsc/bun_string_jsc.rs index 7edd84e60e58..7239540f72b0 100644 --- a/src/jsc/bun_string_jsc.rs +++ b/src/jsc/bun_string_jsc.rs @@ -27,6 +27,15 @@ unsafe extern "C" { safe fn JSC__createError(global: &JSGlobalObject, str_: &String) -> JSValue; safe fn JSC__createTypeError(global: &JSGlobalObject, str_: &String) -> JSValue; safe fn JSC__createRangeError(global: &JSGlobalObject, str_: &String) -> JSValue; + safe fn JSC__createErrorWithLocation( + global: &JSGlobalObject, + error_type: u8, + message: &String, + source_url: &String, + line: i32, + column: i32, + stack: &String, + ) -> JSValue; } // ── bun.String methods ────────────────────────────────────────────────────── @@ -55,6 +64,55 @@ pub(crate) fn to_range_error_instance(this: &String, global_object: &JSGlobalObj result } +/// Builds a `JSErrorCode`-typed ErrorInstance describing a diagnostic that has +/// a source location but no JS frames (a parse or resolve failure). Its `stack` +/// is the conventional `Name: message\n at file:line:column`, so anything +/// that reads `.stack` — structured clone, `console.error`, the uncaught-error +/// printer — sees the diagnostic's location rather than nothing. +pub fn error_instance_from_location( + global: &JSGlobalObject, + code: crate::JSErrorCode, + message: &[u8], + file: &[u8], + line: i32, + column: i32, +) -> JSValue { + use std::io::Write as _; + + // JSC::errorTypeName + let name: &str = match code { + crate::JSErrorCode::EvalError => "EvalError", + crate::JSErrorCode::RangeError => "RangeError", + crate::JSErrorCode::ReferenceError => "ReferenceError", + crate::JSErrorCode::SyntaxError => "SyntaxError", + crate::JSErrorCode::TypeError => "TypeError", + crate::JSErrorCode::URIError => "URIError", + crate::JSErrorCode::AggregateError => "AggregateError", + _ => "Error", + }; + let mut stack: Vec = Vec::with_capacity(name.len() + message.len() + 64); + let _ = write!(&mut stack, "{name}: {}", bstr::BStr::new(message)); + let (line, column) = if file.is_empty() { + (0, 0) + } else { + (line.max(0), column.max(0)) + }; + if !file.is_empty() { + let _ = write!(&mut stack, "\n at {}", bstr::BStr::new(file)); + if line > 0 { + let _ = write!(&mut stack, ":{line}"); + if column > 0 { + let _ = write!(&mut stack, ":{column}"); + } + } + } + + let message = bun_core::OwnedString::new(String::clone_utf8(message)); + let source_url = bun_core::OwnedString::new(String::clone_utf8(file)); + let stack = bun_core::OwnedString::new(String::clone_utf8(&stack)); + JSC__createErrorWithLocation(global, code.0, &message, &source_url, line, column, &stack) +} + #[inline] #[track_caller] pub fn from_js(value: JSValue, global_object: &JSGlobalObject) -> JsResult { diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 114ad0091bff..f92a70f92358 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1168,17 +1168,6 @@ fn on_unhandled_rejection( .to_error() .unwrap_or(error_instance_or_exception); - // A parse failure rejects with a BuildMessage, which doesn't survive structured - // clone. Node reports a SyntaxError; build a real one from the formatted parse - // error so the subtype reaches the parent intact. - if let Some(bm) = error_instance.as_::() { - // SAFETY: as_ returned a live BuildMessage cell, read-only on the - // worker (JS) thread that owns it. - let text = unsafe { (*bm).msg.data.text.clone() }; - error_instance = - global_object.create_syntax_error_instance(format_args!("{}", bstr::BStr::new(&text))); - } - let mut array: Vec = Vec::new(); // `worker_ref()` is the safe BACKREF accessor — `vm.worker` points at the diff --git a/src/runtime/bake/dev_server/error_report_request.rs b/src/runtime/bake/dev_server/error_report_request.rs index 48bd7434f12d..b3e161c1077d 100644 --- a/src/runtime/bake/dev_server/error_report_request.rs +++ b/src/runtime/bake/dev_server/error_report_request.rs @@ -152,7 +152,7 @@ impl ErrorReportRequest { code_type: ZigStackFrameCode::NONE, is_async: false, remapped: false, - jsc_stack_frame_index: -1, + source_provider: None, }); } diff --git a/test/cli/inspect/inspect.test.ts b/test/cli/inspect/inspect.test.ts index 04719466ad3e..49b06d916b41 100644 --- a/test/cli/inspect/inspect.test.ts +++ b/test/cli/inspect/inspect.test.ts @@ -589,17 +589,5 @@ test("error.stack doesnt lose frames", () => { `); // In Bun v1.2.20 and lower, we would only have the first frame here. - expect(yes).toMatchInlineSnapshot(` - " - error: test - at bottom (/inspect.test.ts::) - at middle (/inspect.test.ts::) - at IGNORE_ME_BEFORE_THIS_LINE (/inspect.test.ts::) - at accessErrorStackProperty (/inspect.test.ts::) - at /inspect.test.ts:: - " - `); - - // We allow it to differ by the existence of as a string. But that's it. - expect(no.split("\n").slice(0, -2).join("\n").trim()).toBe(yes.split("\n").slice(0, -2).join("\n").trim()); + expect(yes).toBe(no); }); diff --git a/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap b/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap index 650537102d0a..a9dbd738e57c 100644 --- a/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap +++ b/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap @@ -12,7 +12,10 @@ Error: hello `; exports[`can get sourceURL inside node:vm 1`] = ` -"4 | return Bun.inspect(new Error("hello")); +"1 | +2 | +3 | function hello() { +4 | return Bun.inspect(new Error("hello")); ^ error: hello at hello (hellohello.js:4:24) @@ -22,7 +25,10 @@ error: hello `; exports[`eval sourceURL is correct 1`] = ` -"4 | return Bun.inspect(new Error("hello")); +"1 | +2 | +3 | function hello() { +4 | return Bun.inspect(new Error("hello")); ^ error: hello at hello (hellohello.js:4:24) diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index cc7acb16c723..299da69eff9c 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -296,25 +296,102 @@ test("support require in eval for a file", async () => { test("support require in eval for a file that doesnt exist", async () => { const worker = new Worker(`postMessage(require('./fixture-invalid.js').argv[0])`, { eval: true }); - const result = await new Promise(resolve => { + const result = await new Promise(resolve => { worker.on("message", resolve); worker.on("error", resolve); }); - expect(result.toString()).toInclude(`error: Cannot find module './fixture-invalid.js' from 'blob:`); + expect(result).toBeInstanceOf(Error); + expect(result.message).toStartWith(`Cannot find module './fixture-invalid.js'\nRequire stack:\n- blob:`); await worker.terminate(); }); -test("support worker eval that throws", async () => { +test("worker eval parse errors reach the parent as a SyntaxError with its location", async () => { const worker = new Worker(`postMessage(throw new Error("boom"))`, { eval: true }); - const result = await new Promise(resolve => { + const result = await new Promise(resolve => { worker.on("message", resolve); worker.on("error", resolve); }); - expect(result.toString()).toInclude("Unexpected throw"); - expect(result.name).toBe("SyntaxError"); + expect({ + constructor: result?.constructor, + message: result?.message, + line: result?.line, + column: result?.column, + stack: typeof result?.stack, + }).toEqual({ + constructor: SyntaxError, + message: "Unexpected throw", + line: 1, + column: 13, + stack: "string", + }); + expect(result.stack).toMatch(/^SyntaxError: Unexpected throw\n at .+:1:13$/); await worker.terminate(); }); +test("worker entry-point parse errors reach the parent as a SyntaxError with its location", async () => { + using dir = tempDir("worker-syntax-error", { + "bad.js": "// line 1\nconst y = ;\n", + "main-listener.mjs": ` + import { Worker } from "node:worker_threads"; + const worker = new Worker(new URL("./bad.js", import.meta.url)); + const e = await new Promise(r => { worker.on("message", r); worker.on("error", r); }); + console.log(JSON.stringify({ + constructor: e?.constructor?.name, + message: e?.message, + line: e?.line, + column: e?.column, + sourceURL: String(e?.sourceURL).replaceAll("\\\\", "/"), + stack: String(e?.stack).replaceAll("\\\\", "/"), + })); + `, + "main-uncaught.mjs": ` + import { Worker } from "node:worker_threads"; + const worker = new Worker(new URL("./bad.js", import.meta.url)); + await new Promise(r => worker.on("exit", r)); + `, + }); + const badPath = join(String(dir), "bad.js").replaceAll("\\", "/"); + + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "main-listener.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "inherit", + }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + expect(JSON.parse(stdout)).toEqual({ + constructor: "SyntaxError", + message: "Unexpected ;", + line: 2, + column: 11, + sourceURL: badPath, + stack: `SyntaxError: Unexpected ;\n at ${badPath}:2:11`, + }); + expect(exitCode).toBe(0); + } + + // With no 'error' listener the parent dies with the worker's error; what it + // prints must point at the worker file, not at node:events' rethrow. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "main-uncaught.mjs"], + env: { ...bunEnv, NO_COLOR: "1" }, + cwd: String(dir), + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const output = stderr.replaceAll("\\", "/"); + expect(output).toContain("2 | const y = ;"); + expect(output).toContain("SyntaxError: Unexpected ;"); + expect(output).toContain(`at ${badPath}:2:11`); + expect(output).not.toContain("node:events"); + expect(exitCode).toBe(1); + } +}); + describe("execArgv option", async () => { // this needs to be a subprocess to ensure that the parent's execArgv is not empty // otherwise we could not distinguish between the worker inheriting the parent's execArgv diff --git a/test/js/web/workers/structured-clone.test.ts b/test/js/web/workers/structured-clone.test.ts index d78df66bcd49..121e89e3b416 100644 --- a/test/js/web/workers/structured-clone.test.ts +++ b/test/js/web/workers/structured-clone.test.ts @@ -1,6 +1,6 @@ import { deserialize, serialize } from "bun:jsc"; import { openSync } from "fs"; -import { bunEnv, bunExe, tls } from "harness"; +import { bunEnv, bunExe, tempDir, tls } from "harness"; import { createPrivateKey, createPublicKey, createSecretKey, KeyObject, X509Certificate } from "node:crypto"; import { BlockList } from "node:net"; import { deflate } from "node:zlib"; @@ -936,6 +936,70 @@ describe("Error serialization semantics", () => { Error.prepareStackTrace = original; } }); + + // Bun's parse/resolve diagnostics wrap VM-local state; they cross a + // structured-clone boundary as the plain error carrying message + location. + describe("Bun diagnostics", () => { + const dir = tempDir("structured-clone-diagnostics", { + "bad-syntax.js": "// line 1\nconst y = ;\n", + "bad-import.js": "// line 1\nimport 'this-package-does-not-exist';\n", + }); + afterAll(() => dir[Symbol.dispose]()); + const badSyntax = join(String(dir), "bad-syntax.js"); + const badImport = join(String(dir), "bad-import.js"); + const slashes = (s: unknown) => String(s).replaceAll("\\", "/"); + + test("BuildMessage clones as a SyntaxError with the parse error's location", async () => { + const buildMessage: any = await import(badSyntax).then( + () => expect.unreachable(), + e => e, + ); + expect(buildMessage.constructor.name).toBe("BuildMessage"); + for (const clone of [structuredClone, jscSerializeRoundtrip]) { + const cloned = clone(buildMessage); + expect({ + constructor: cloned.constructor, + message: cloned.message, + sourceURL: slashes(cloned.sourceURL), + line: cloned.line, + column: cloned.column, + stack: slashes(cloned.stack), + }).toEqual({ + constructor: SyntaxError, + message: buildMessage.message, + sourceURL: slashes(badSyntax), + line: 2, + column: 11, + stack: `SyntaxError: ${buildMessage.message}\n at ${slashes(badSyntax)}:2:11`, + }); + } + // Still enters the object pool: duplicate references keep their identity. + const [a, b] = structuredClone([buildMessage, buildMessage]); + expect(a).toBe(b); + }); + + test("ResolveMessage clones as an Error pointing at the importer", async () => { + const resolveMessage: any = await import(badImport).then( + () => expect.unreachable(), + e => e, + ); + expect(resolveMessage.constructor.name).toBe("ResolveMessage"); + for (const clone of [structuredClone, jscSerializeRoundtrip]) { + const cloned = clone(resolveMessage); + expect({ + constructor: cloned.constructor, + message: cloned.message, + sourceURL: slashes(cloned.sourceURL), + stack: slashes(cloned.stack), + }).toEqual({ + constructor: Error, + message: resolveMessage.message, + sourceURL: slashes(badImport), + stack: `Error: ${slashes(resolveMessage.message)}\n at ${slashes(badImport)}`, + }); + } + }); + }); }); describe("options.transfer iterator error propagation", () => { diff --git a/test/regression/issue/23022-stack-trace-iterator.test.ts b/test/regression/issue/23022-stack-trace-iterator.test.ts index 7ff0803ae945..dfb1ea328640 100644 --- a/test/regression/issue/23022-stack-trace-iterator.test.ts +++ b/test/regression/issue/23022-stack-trace-iterator.test.ts @@ -27,7 +27,8 @@ test("V8StackTraceIterator handles frames without parentheses (issue #23022)", a const stackFrames = err.stack?.split("\n").filter(line => line.trim().startsWith("at")); expect(stackFrames?.length).toBeGreaterThan(3); - // Ensure both "unknown" frames and regular frames are present - expect(inspected).toContain("at unknown"); + // https://github.com/oven-sh/bun/issues/23022: frames after the `at unknown` placeholder must survive; the placeholder itself is skipped. expect(inspected).toContain("at _write"); + expect(inspected.replaceAll("\\", "/")).toContain(import.meta.path.replaceAll("\\", "/")); + expect(inspected).not.toContain("at unknown"); });