Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
26 changes: 26 additions & 0 deletions src/http/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,32 @@ impl bun_core::output::ErrName for Error {
}
}

impl Error {
/// Node.js / undici-compatible errno-style code string for `err.code`.
/// Falls back to the Bun label for variants with no established mapping.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[allow(clippy::trivially_copy_pass_by_ref)]
pub fn errno_code(&self) -> &'static str {
match self {
Self::ConnectionRefused => "ECONNREFUSED",
Self::ConnectionClosed => "ECONNRESET",
Self::Timeout => "ETIMEDOUT",
Self::InvalidHTTPResponse => "HPE_INVALID_CHUNK_SIZE",
Self::ResponseHeadersTooLarge => "UND_ERR_HEADERS_OVERFLOW",
Self::InvalidContentLength => "UND_ERR_RES_CONTENT_LENGTH_MISMATCH",
Self::HTTP2ContentLengthMismatch | Self::HTTP3ContentLengthMismatch => {
"UND_ERR_RES_CONTENT_LENGTH_MISMATCH"
}
Self::Zlib(_) => "Z_DATA_ERROR",
Comment thread
robobun marked this conversation as resolved.
Self::Picohttp(bun_picohttp::ParseResponseError::MalformedHttpResponse) => {
"HPE_INVALID_CONSTANT"
}
Comment thread
robobun marked this conversation as resolved.
Self::Cert(e) => <&'static str>::from(e),
Self::Sys(e) => <&'static str>::from(e),
_ => self.name(),
}
}
}

impl From<bun_zlib::ZlibError> for Error {
fn from(e: bun_zlib::ZlibError) -> Self {
match e {
Expand Down
56 changes: 56 additions & 0 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2353,6 +2353,62 @@ JSC::EncodedJSValue JSGlobalObject__createOutOfMemoryError(JSC::JSGlobalObject*
return JSValue::encode(exception);
}

// Create a bare Error whose only purpose is to snapshot the caller's
// synchronous stack. Used by fetch() to record the call site so the later
// event-loop-created rejection error can point at user code.
Comment thread
robobun marked this conversation as resolved.
Outdated
extern "C" JSC::EncodedJSValue Bun__captureCallerStackError(JSC::JSGlobalObject* globalObject)
{
return JSC::JSValue::encode(JSC::createError(globalObject, "fetch"_s));
}

// Wrap a fetch network error as the WHATWG / undici shape: an outer
// TypeError('fetch failed' | 'terminated') carrying the underlying error as
// `.cause`. The outer error also gets `.code` mirrored from the cause so
// existing `err.code === "ECONNREFUSED"` checks keep working.
//
// `stackSourceValue` is an ErrorInstance captured at the original fetch()
// call site; its frames are transplanted onto the new TypeError so `.stack`
// points at the caller instead of being empty (the TypeError is created from
// an event-loop task where the interpreter stack is empty).
Comment thread
robobun marked this conversation as resolved.
Outdated
extern "C" JSC::EncodedJSValue Bun__createFetchFailedTypeError(
JSC::JSGlobalObject* globalObject,
JSC::EncodedJSValue causeValue,
JSC::EncodedJSValue stackSourceValue,
bool terminated)
{
auto& vm = JSC::getVM(globalObject);
auto clientData = WebCore::clientData(vm);

JSC::JSObject* result = JSC::createTypeError(globalObject,
terminated ? "terminated"_s : "fetch failed"_s);

JSC::JSValue cause = JSC::JSValue::decode(causeValue);
if (cause && !cause.isEmpty() && !cause.isUndefined()) {
result->putDirect(vm, vm.propertyNames->cause, cause, JSC::PropertyAttribute::DontEnum | 0);

if (auto* causeObj = cause.getObject()) {
JSC::JSValue code = causeObj->getDirect(vm, clientData->builtinNames().codePublicName());
if (code && !code.isUndefined())
result->putDirect(vm, clientData->builtinNames().codePublicName(), code, 0);
}
}

JSC::JSValue stackSrc = JSC::JSValue::decode(stackSourceValue);
if (auto* srcInstance = dynamicDowncast<JSC::ErrorInstance>(stackSrc)) {
if (auto* destInstance = dynamicDowncast<JSC::ErrorInstance>(result)) {
if (auto* srcTrace = srcInstance->stackTrace(); srcTrace && !srcTrace->isEmpty()) {
// Copy (not move): the source may be shared via ValueError::dupe
// when a Response body is cloned.
Comment thread
robobun marked this conversation as resolved.
Outdated
WTF::Vector<JSC::StackFrame> frames;
frames.appendVector(*srcTrace);
destInstance->setStackFrames(vm, WTF::move(frames));
}
Comment thread
robobun marked this conversation as resolved.
}
}

return JSC::JSValue::encode(result);
}

JSC::EncodedJSValue SystemError__toErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject)
{
SystemError err = *arg0;
Expand Down
10 changes: 10 additions & 0 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,15 @@ impl From<JsError> for bun_event_loop::ErasedJsError {
}
}

/// Create a bare `Error` whose only purpose is to snapshot the current JS
/// stack. Used by `fetch()` to record the call site while the caller is still
/// on the stack; the frames are later transplanted onto the rejection error
/// (which is minted from an event-loop task with an empty interpreter stack).
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
pub fn capture_caller_stack_error(global: &JSGlobalObject) -> JSValue {
Bun__captureCallerStackError(global)
}

/// Converts `bun.JSError` → `std.Io.Writer.Error` for Console formatting paths.
/// `Display` impls return `fmt::Error`; the JS exception, if any, remains on the VM.
#[inline]
Expand Down Expand Up @@ -1170,6 +1179,7 @@ unsafe extern "C" {
global: &JSGlobalObject,
code: u8,
) -> JSValue;
safe fn Bun__captureCallerStackError(global: &JSGlobalObject) -> JSValue;
safe fn ZigString__toValueGC(this: &bun_core::ZigString, global: &JSGlobalObject) -> JSValue;
// ZigString__toExternalValue: use the generated `cpp::` re-export (canonical signature).
safe fn ZigString__toJSONObject(this: &bun_core::ZigString, global: &JSGlobalObject)
Expand Down
45 changes: 45 additions & 0 deletions src/runtime/webcore/Body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,31 @@ pub enum ValueError {
/// error" to TypeError, so use this for fetch-layer rejections that
/// callers feature-detect via `err instanceof TypeError`.
TypeError(BunString),
/// WHATWG fetch "network error": an outer `TypeError('fetch failed')`
/// (or `'terminated'` once the response body is streaming) whose `.cause`
/// is the underlying system error. `stack_source` carries the caller's
/// synchronous stack captured at the original `fetch()` call so the
/// rejection (minted at event-loop top with an empty interpreter stack)
/// still points at user code.
Comment thread
robobun marked this conversation as resolved.
Outdated
FetchFailed {
cause: SystemError,
terminated: bool,
stack_source: jsc::strong::Optional,
},
JSValue(jsc::strong::Optional),
}

// SAFETY: `JSGlobalObject` is opaque `UnsafeCell`, so `&JSGlobalObject` is
// ABI-identical to a non-null `JSGlobalObject*`; `JSValue` is `#[repr(C)]`.
unsafe extern "C" {
safe fn Bun__createFetchFailedTypeError(
global: &JSGlobalObject,
cause: JSValue,
stack_source: JSValue,
terminated: bool,
) -> JSValue;
}

impl ValueError {
// Not a clean Drop — resets self to safe-empty in place. Renamed from `deinit`
// per PORTING.md (never expose `pub fn deinit(&mut self)`).
Expand All @@ -581,6 +603,7 @@ impl ValueError {
ValueError::SystemError(_system_error) => {}
ValueError::Message(message) => message.deref(),
ValueError::TypeError(message) => message.deref(),
ValueError::FetchFailed { stack_source, .. } => stack_source.deinit(),
ValueError::JSValue(v) => v.deinit(),
ValueError::AbortReason(_) => {}
}
Expand Down Expand Up @@ -614,6 +637,16 @@ impl ValueError {
}
ValueError::Message(message) => message.to_error_instance(global_object),
ValueError::TypeError(message) => message.to_type_error_instance(global_object),
ValueError::FetchFailed {
cause,
terminated,
stack_source,
} => {
let cause_js = core::mem::take(cause).to_error_instance(global_object);
cause_js.ensure_still_alive();
let stack_src = stack_source.swap();
Bun__createFetchFailedTypeError(global_object, cause_js, stack_src, *terminated)
}
// do an early return in this case we don't need to create a new Strong
ValueError::JSValue(js_value) => {
return js_value.get().unwrap_or(JSValue::UNDEFINED);
Expand All @@ -630,6 +663,18 @@ impl ValueError {
ValueError::SystemError(e) => ValueError::SystemError(e.clone()),
ValueError::Message(m) => ValueError::Message(m.clone()),
ValueError::TypeError(m) => ValueError::TypeError(m.clone()),
ValueError::FetchFailed {
cause,
terminated,
stack_source,
} => ValueError::FetchFailed {
cause: cause.clone(),
terminated: *terminated,
stack_source: match stack_source.get() {
Some(v) => jsc::strong::Optional::create(v, global_object),
None => jsc::strong::Optional::empty(),
},
},
ValueError::JSValue(js_ref) => {
if let Some(js_value) = js_ref.get() {
return ValueError::JSValue(jsc::strong::Optional::create(
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2097,6 +2097,10 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
} else {
jsc::strong::Optional::create(check_server_identity, global_this)
},
caller_stack_source: jsc::strong::Optional::create(
jsc::capture_caller_stack_error(global_this),
global_this,
),
unix_socket_path: core::mem::replace(&mut unix_socket_path, ZigStringSlice::empty()),
};

Expand Down
34 changes: 27 additions & 7 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ pub struct FetchTasklet {
// must be stored because AbortSignal stores reason weakly
pub abort_reason: StrongOptional,

/// ErrorInstance created at the JS-thread `fetch()` call purely to snapshot
/// the caller's synchronous stack. Transplanted onto the rejection
/// `TypeError('fetch failed')`, which is otherwise stackless (minted from
/// an event-loop task with an empty interpreter stack).
Comment thread
robobun marked this conversation as resolved.
Outdated
pub caller_stack_source: StrongOptional,

// custom checkServerIdentity
pub check_server_identity: StrongOptional,
pub reject_unauthorized: bool,
Expand Down Expand Up @@ -486,6 +492,7 @@ impl FetchTasklet {
self.request_body.detach();

self.abort_reason.deinit();
self.caller_stack_source.deinit();
self.check_server_identity.deinit();
self.clear_abort_signal();
// Clear the sink only after the requested ended otherwise we would potentialy lose the last chunk
Expand Down Expand Up @@ -1304,6 +1311,12 @@ impl FetchTasklet {
BunString::EMPTY
};

// An error after response headers arrived surfaces on the body reader,
// where undici uses `TypeError('terminated')`; before headers the fetch
// promise itself rejects with `TypeError('fetch failed')`.
Comment thread
robobun marked this conversation as resolved.
Outdated
let terminated = self.metadata.is_some();
let stack_source = core::mem::take(&mut self.caller_stack_source);
Comment thread
claude[bot] marked this conversation as resolved.

// The hostname never resolved: report the resolver error (`ENOTFOUND`,
// ...) with `syscall`/`hostname`, the same shape `node:dns` produces,
// rather than a generic connect-failure message. `dns_error` is the
Expand All @@ -1322,15 +1335,15 @@ impl FetchTasklet {
hostname,
);
err.path = path.into();
return BodyValueError::SystemError(err);
return BodyValueError::FetchFailed {
cause: err,
terminated,
stack_source,
};
}
}

let code = if fail == http::Error::ConnectionClosed {
BunString::static_("ECONNRESET")
} else {
BunString::static_(fail.name())
};
let code = BunString::static_(fail.errno_code());

let message = match fail {
http::Error::ConnectionClosed => BunString::static_(
Expand Down Expand Up @@ -1562,7 +1575,11 @@ impl FetchTasklet {
..Default::default()
};

BodyValueError::SystemError(fetch_error)
BodyValueError::FetchFailed {
cause: fetch_error,
terminated,
stack_source,
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}

pub(crate) fn on_readable_stream_available(
Expand Down Expand Up @@ -1897,6 +1914,7 @@ impl FetchTasklet {
signal_store: http::signals::Store::default(),
has_schedule_callback: AtomicBool::new(false),
abort_reason: StrongOptional::empty(),
caller_stack_source: fetch_options.caller_stack_source,
check_server_identity: fetch_options.check_server_identity,
reject_unauthorized: fetch_options.reject_unauthorized,
upgraded_connection: fetch_options.upgraded_connection,
Expand Down Expand Up @@ -2564,6 +2582,7 @@ pub struct FetchOptions {
// Custom Hostname
pub hostname: Option<Box<[u8]>>,
pub check_server_identity: StrongOptional,
pub caller_stack_source: StrongOptional,
pub unix_socket_path: ZigStringSlice,
pub ssl_config: Option<http::ssl_config::SharedPtr>,
pub upgraded_connection: bool,
Expand Down Expand Up @@ -2600,6 +2619,7 @@ impl Default for FetchOptions {
global_this: None,
hostname: None,
check_server_identity: StrongOptional::empty(),
caller_stack_source: StrongOptional::empty(),
unix_socket_path: ZigStringSlice::EMPTY,
ssl_config: None,
upgraded_connection: false,
Expand Down
4 changes: 2 additions & 2 deletions test/bake/fixtures/deinitialization/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async function run({ closeActiveConnections = false, sendAnyRequests = true, web

if (sendAnyRequests) {
if (closeActiveConnections) {
expect(fetch(server.url.origin, { keepalive: false })).rejects.toThrow("closed unexpectedly");
expect(fetch(server.url.origin, { keepalive: false })).rejects.toThrow("fetch failed");
} else {
const response = await fetch(server.url.origin, { keepalive: false });
expect(response.status).toBe(200);
Expand All @@ -68,7 +68,7 @@ async function run({ closeActiveConnections = false, sendAnyRequests = true, web
}

// Server is closed
expect(fetch(server.url.origin, { keepalive: false })).rejects.toThrow("Unable to connect");
expect(fetch(server.url.origin, { keepalive: false })).rejects.toThrow("fetch failed");
Comment thread
robobun marked this conversation as resolved.
Outdated
}

try {
Expand Down
4 changes: 2 additions & 2 deletions test/js/bun/http/bun-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,12 @@ describe.concurrent("Server", () => {
{
expect(
async () => await fetch(server.url, { tls: { rejectUnauthorized: true } }).then(res => res.text()),
).toThrow("self signed certificate");
).toThrow("fetch failed");
}

{
using _ = rejectUnauthorizedScope(true);
expect(async () => await fetch(server.url).then(res => res.text())).toThrow("self signed certificate");
expect(async () => await fetch(server.url).then(res => res.text())).toThrow("fetch failed");
Comment thread
robobun marked this conversation as resolved.
Outdated
}

{
Expand Down
11 changes: 8 additions & 3 deletions test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ it("should be able to abruptly stop the server many times", async () => {
await fetch(url, { keepalive: true }).then(res => res.text());
expect.unreachable();
} catch (e) {
expect(["ECONNRESET", "ConnectionRefused"]).toContain(e.code);
expect(["ECONNRESET", "ECONNREFUSED"]).toContain(e.code);
}
}

Expand Down Expand Up @@ -2481,7 +2481,12 @@ it.concurrent("should not instanciate error instances in each request", async ()
await Promise.all(batch);
}
}
expect(heapStats().objectTypeCounts.Error || 0).toBeLessThanOrEqual(startErrorCount);
// fetch() allocates one Error per call to capture the caller's stack for the
// rejection path; it is released with the tasklet. GC first so those
// transient instances don't count against the server-side regression this
// test guards.
Bun.gc(true);
expect(heapStats().objectTypeCounts.Error || 0).toBeLessThanOrEqual(startErrorCount + batchSize);
Comment thread
robobun marked this conversation as resolved.
Outdated
});

it("should be able to abort a sendfile response and streams", async () => {
Expand Down Expand Up @@ -2676,7 +2681,7 @@ it.concurrent(
if (success) {
expect(res.text()).resolves.toBe("Hello, World!");
} else {
expect(res.text()).rejects.toThrow(/The socket connection was closed unexpectedly./);
expect(res.text()).rejects.toMatchObject({ name: "TypeError", code: "ECONNRESET" });
}
}
await Promise.all([testTimeout("/ok", true), testTimeout("/timeout", false)]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ try {
await res.text();
expect(true).toBe("unreacheable");
} catch (err) {
expect(err.name).toBe("TypeError");
expect(err.message).toBe("fetch failed");
expect(err.code).toBe("FailedToOpenSocket");
expect(err.message).toBe("Was there a typo in the url or port?");
expect(err.cause?.message).toBe("Was there a typo in the url or port?");
Comment thread
robobun marked this conversation as resolved.
}
Loading
Loading