Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
18 changes: 10 additions & 8 deletions src/jsc/bindings/AsyncStackTrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "BunClientData.h"
#include "ErrorStackFrame.h"
#include "FormatStackTraceForJS.h"

#include <JavaScriptCore/CodeBlock.h>
#include <JavaScriptCore/ErrorInstance.h>
Expand Down Expand Up @@ -166,13 +167,14 @@ extern "C" void Bun__attachAsyncStackFromPromise(JSC::JSGlobalObject* globalObje
return;

size_t limit = globalObject->stackTraceLimit().value_or(10);
if (!limit)
return;

WTF::Vector<JSC::StackFrame> frames;
collectAsyncStackFramesFromPromise(vm, instance, promise, frames, limit);
if (frames.isEmpty())
return;
if (limit) {
WTF::Vector<JSC::StackFrame> frames;
collectAsyncStackFramesFromPromise(vm, instance, promise, frames, limit);
if (!frames.isEmpty()) {
instance->setStackFrames(vm, WTF::move(frames));
return;
}
}

instance->setStackFrames(vm, WTF::move(frames));
Bun::installLazyStackIfFrameless(vm, globalObject, instance);
}
3 changes: 3 additions & 0 deletions src/jsc/bindings/ErrorCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <openssl/err.h>
#include "ErrorCode.h"
#include "ErrorStackTrace.h"
#include "FormatStackTraceForJS.h"
#include "KeyObject.h"

namespace WTF {
Expand Down Expand Up @@ -216,6 +217,8 @@ JSObject* ErrorCodeCache::createError(VM& vm, Zig::GlobalObject* globalObject, E
// exception were thrown by ErrorInstance::create)
return uncheckedDowncast<JSObject>(thrown_exception->value());
}
// Native code also builds these from I/O callbacks (e.g. the redis client), with no JS on the stack.
Bun::installLazyStackIfFrameless(vm, globalObject, created_error);
return created_error;
}

Expand Down
16 changes: 16 additions & 0 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,22 @@ JSC_DEFINE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter, (JSGlobalObject * g
return true;
}

void installLazyStackIfFrameless(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::ErrorInstance* error)
{
// ErrorInstance::materializeErrorInfoIfNeeded does nothing for an empty trace. A null trace
// means Error.stackTraceLimit was deleted, which leaves .stack undefined in V8 as well.
Comment thread
robobun marked this conversation as resolved.
Outdated
auto* stackTrace = error->stackTrace();
if (!stackTrace || !stackTrace->isEmpty())
return;

// Already installed, or assigned explicitly.
if (error->getDirect(vm, vm.propertyNames->stack))
return;

auto* globalObject = defaultGlobalObject(lexicalGlobalObject);
error->putDirectCustomAccessor(vm, vm.propertyNames->stack, globalObject->m_lazyStackCustomGetterSetter.get(globalObject), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::CustomAccessor | 0);
}

JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame))
{
Zig::GlobalObject* globalObject = static_cast<Zig::GlobalObject*>(lexicalGlobalObject);
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/FormatStackTraceForJS.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionDefaultErrorPrepareStackTrace);
JSC_DECLARE_CUSTOM_GETTER(errorInstanceLazyStackCustomGetter);
JSC_DECLARE_CUSTOM_SETTER(errorInstanceLazyStackCustomSetter);

// An error created while no JS is running captures no frames; like V8, still give it a
// .stack that formats to "Name: message" on first access.
Comment thread
robobun marked this conversation as resolved.
Outdated
void installLazyStackIfFrameless(JSC::VM& vm, JSC::JSGlobalObject* lexicalGlobalObject, JSC::ErrorInstance* error);

// Internal wrapper functions for JSC error info callbacks
WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, WTF::Vector<JSC::StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, WTF::String& sourceURL, void* bunErrorData);
JSC::JSValue computeErrorInfoWrapperToJSValue(JSC::VM& vm, WTF::Vector<JSC::StackFrame>& stackTrace, unsigned int& line_in, unsigned int& column_in, WTF::String& sourceURL, JSC::JSObject* errorInstance, void* bunErrorData);
Expand Down
3 changes: 3 additions & 0 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
#include "ErrorStackFrame.h"
#include "AsyncStackTrace.h"
#include "ErrorStackTrace.h"
#include "FormatStackTraceForJS.h"
#include "ObjectBindings.h"

#include <JavaScriptCore/VMInlines.h>
Expand Down Expand Up @@ -2583,6 +2584,8 @@ static JSC::EncodedJSValue systemErrorToErrorInstance(const SystemError* arg0, J
auto& names = WebCore::builtinNames(vm);

JSC::JSObject* result = createError(globalObject, errorType, message);
// Usually created from an I/O completion callback, with no JS on the stack.
Bun::installLazyStackIfFrameless(vm, globalObject, uncheckedDowncast<JSC::ErrorInstance>(result));

auto clientData = WebCore::clientData(vm);

Expand Down
21 changes: 21 additions & 0 deletions test/js/bun/util/password.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,27 @@ test("verify rejects encoded argon2 hashes with cost parameters above the suppor
await expect(password.verify("correct horse", junkMemory)).rejects.toThrow("InvalidEncoding");
});

test("verify's rejection has a .stack even when nothing awaits it", async () => {
// The error is created once the thread pool job completes, with no JS on the stack, and
// .then() leaves no await chain to recover frames from.
const err = await new Promise<any>(resolve => password.verify("correct horse", "not a hash").then(resolve, resolve));

expect(err.code).toBe("PASSWORD_UNSUPPORTED_ALGORITHM");
expect(err.stack).toBe(`${err.name}: ${err.message}`);
});

test("verify's rejection lists the async frame when awaited inside an async function", async () => {
async function check() {
await password.verify("correct horse", "not a hash");
}
const err = await check().then(
() => new Error("unexpected resolve"),
e => e,
);

expect(err.stack).toStartWith(`${err.name}: ${err.message}\n at async check `);
});

test("verifySync reads the password buffer only after every argument has been coerced", () => {
const hashed = password.hashSync("correct horse", { algorithm: "argon2id", memoryCost: 8, timeCost: 1 });
const passwordBytes = new TextEncoder().encode("correct horse");
Expand Down
24 changes: 18 additions & 6 deletions test/js/node/fs/promises.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ test("fs.promises async stack through Promise subclass", async () => {

expect(caught).toBeDefined();
expect(caught.code).toBe("ENOENT");
// Subclass .then() may not preserve the reaction chain — must not crash.
expect(typeof caught.stack === "string" || caught.stack === undefined).toBe(true);
// Subclass .then() may not preserve the reaction chain; the error still gets a .stack.
expect(caught.stack).toStartWith(`Error: ${caught.message}`);
});

test("fs.promises async stack through custom thenable", async () => {
Expand All @@ -282,8 +282,8 @@ test("fs.promises async stack through custom thenable", async () => {

expect(caught).toBeDefined();
expect(caught.code).toBe("ENOENT");
// Custom thenables break the direct reaction chain — must not crash.
expect(typeof caught.stack === "string" || caught.stack === undefined).toBe(true);
// Custom thenables break the direct reaction chain; the error still gets a .stack.
expect(caught.stack).toStartWith(`Error: ${caught.message}`);
});

test("fs.promises async stack with Promise.all", async () => {
Expand All @@ -300,8 +300,20 @@ test("fs.promises async stack with Promise.all", async () => {

expect(caught).toBeDefined();
expect(caught.code).toBe("ENOENT");
// Promise.all uses combinator context — must not crash.
expect(typeof caught.stack === "string" || caught.stack === undefined).toBe(true);
// Promise.all uses combinator context, so no async frames are recovered; the error
// still gets a .stack.
expect(caught.stack).toStartWith(`Error: ${caught.message}`);
});

test("fs.promises errors that nothing awaits still have a .stack", async () => {
// .catch() attaches a handler but nothing awaits the derived promise, so there is no
// async chain to recover frames from: the error keeps an empty trace.
const caught = await new Promise(resolve => readFile("/nonexistent-path/x.txt").catch(resolve));

expect(caught.code).toBe("ENOENT");
expect(caught.stack).toBe(`Error: ${caught.message}`);
expect(Object.prototype.hasOwnProperty.call(caught, "stack")).toBe(true);
expect(Object.keys(caught)).not.toContain("stack");
});

it("an unused FileHandle.writer() does not prevent close()", async () => {
Expand Down
150 changes: 148 additions & 2 deletions test/js/node/v8/capture-stack-trace.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { nativeFrameForTesting } from "bun:internal-for-testing";
import { noInline } from "bun:jsc";
import { afterEach, expect, mock, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { afterEach, describe, expect, mock, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import { once } from "node:events";
import fs from "node:fs";
import net from "node:net";
const origPrepareStackTrace = Error.prepareStackTrace;
afterEach(() => {
Error.prepareStackTrace = origPrepareStackTrace;
Expand Down Expand Up @@ -1121,3 +1124,146 @@ test("lazy error-info materialization does not store an empty stack value when t
});
expect(exitCode).toBe(0);
});

describe("errors created by native code while no JS is running", () => {
// fs callbacks run from the event loop, so the ENOENT error is built with no JS frames
// on the stack and JSC captures an empty trace for it.
async function framelessError() {
using dir = tempDir("frameless-error", {});
const { promise, resolve } = Promise.withResolvers();
fs.readFile(join(String(dir), "missing.txt"), resolve);
const err = await promise;
expect(err.code).toBe("ENOENT");
return err;
}

test("have an own, non-enumerable .stack holding the 'name: message' line", async () => {
const err = await framelessError();
expect(Object.getOwnPropertyDescriptor(err, "stack")).toMatchObject({ enumerable: false, configurable: true });
expect(Object.keys(err)).not.toContain("stack");
expect(err.stack).toBe(`Error: ${err.message}`);
expect(Object.getOwnPropertyDescriptor(err, "stack")).toMatchObject({
value: `Error: ${err.message}`,
writable: true,
enumerable: false,
});
});

test(".stack is formatted on first read, like an error with frames", async () => {
const err = await framelessError();
err.name = "Renamed";
err.message = "changed";
expect(err.stack).toBe("Renamed: changed");
});

test("Error.prepareStackTrace runs on first read and receives no call sites", async () => {
const err = await framelessError();
Error.prepareStackTrace = mock((error, callSites) => `prepared ${error.code} with ${callSites.length} call sites`);
expect(err.stack).toBe("prepared ENOENT with 0 call sites");
expect(err.stack).toBe("prepared ENOENT with 0 call sites");
expect(Error.prepareStackTrace).toHaveBeenCalledTimes(1);
});

test("assigning .stack before it is read replaces it", async () => {
const err = await framelessError();
err.stack = "mine";
expect(Object.getOwnPropertyDescriptor(err, "stack")).toMatchObject({ value: "mine", enumerable: false });
});

// fetch() builds its error when the connection attempt fails and recovers frames, if any,
// from the async functions awaiting the promise it is about to reject.
async function closedPortURL() {
const listener = net.createServer();
await once(listener.listen(0, "127.0.0.1"), "listening");
const { port } = listener.address();
await new Promise(resolve => listener.close(resolve));
return `http://127.0.0.1:${port}/`;
}

test("a rejection nothing awaits gets the 'name: message' line", async () => {
const url = await closedPortURL();
const err = await new Promise(resolve => fetch(url).then(resolve, resolve));

expect(err).toBeInstanceOf(Error);
expect(err.stack).toBe(`${err.name}: ${err.message}`);
});

test("a rejection awaited inside an async function still lists the async frame", async () => {
const url = await closedPortURL();
async function requestIt() {
await fetch(url);
}
const err = await requestIt().then(
() => new Error("unexpected response"),
e => e,
);

expect(err.stack).toStartWith(`${err.name}: ${err.message}\n at async requestIt `);
});

test("Error.stackTraceLimit = 0 leaves the 'name: message' line, as in V8", () => {
using dir = tempDir("frameless-error", {});
const originalLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
let systemError, codeError;
try {
try {
fs.readFileSync(join(String(dir), "missing.txt"));
} catch (e) {
systemError = e;
}
try {
Buffer.alloc(-1);
} catch (e) {
codeError = e;
}
} finally {
Error.stackTraceLimit = originalLimit;
}
expect({ code: systemError.code, stack: systemError.stack }).toEqual({
code: "ENOENT",
stack: `Error: ${systemError.message}`,
});
expect(codeError.code).toBe("ERR_OUT_OF_RANGE");
expect(codeError.stack).toStartWith("RangeError");
expect(codeError.stack).toEndWith(`: ${codeError.message}`);
});

test("deleting Error.stackTraceLimit still disables .stack entirely, as in V8", () => {
using dir = tempDir("frameless-error", {});
const originalLimit = Error.stackTraceLimit;
delete Error.stackTraceLimit;
let systemError;
try {
try {
fs.readFileSync(join(String(dir), "missing.txt"));
} catch (e) {
systemError = e;
}
} finally {
Error.stackTraceLimit = originalLimit;
}
expect({ code: systemError.code, stack: systemError.stack }).toEqual({ code: "ENOENT", stack: undefined });
});

// node's unhandled rejection warning only prints the rejection value's .stack when the
// value has an own .stack property; without one it falls back to a generic rendering.
test("--unhandled-rejections=warn prints the error for a native rejection", async () => {
using dir = tempDir("frameless-error", {});
await using proc = Bun.spawn({
cmd: [
bunExe(),
"--unhandled-rejections=warn",
"-e",
`require("node:fs/promises").readFile(${JSON.stringify(join(String(dir), "missing.txt"))})`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory");
expect(stderr).not.toContain("[object Object]");
expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 0 });
});
});
22 changes: 22 additions & 0 deletions test/js/valkey/reliability/connection-failures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,3 +441,25 @@ describe("Valkey: Auto-Reconnect In-Flight Commands", () => {
}
});
});

describe("Valkey: Connection Error Shape", () => {
test("a command rejected because the connection failed has a .stack", async () => {
const listener = net.createServer();
await new Promise<void>(resolve => listener.listen(0, "127.0.0.1", resolve));
const { port } = listener.address() as net.AddressInfo;
await new Promise(resolve => listener.close(resolve));

const client = new RedisClient(`redis://127.0.0.1:${port}`, { autoReconnect: false });
try {
// The error is created by the socket's failure callback, with no JS on the stack, and
// .then() leaves no await chain to recover frames from.
const err = await new Promise<any>(resolve => client.get("key").then(resolve, resolve));

expect(err.code).toBe("ERR_REDIS_CONNECTION_CLOSED");
expect(err.stack).toStartWith(err.name);
expect(err.stack).toEndWith(`: ${err.message}`);
} finally {
client.close();
}
});
});
Loading