Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 17 additions & 3 deletions src/bun.js/bindings/ErrorCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1642,11 +1642,25 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionMakeAbortError, (JSC::JSGlobalObject * lexica
return JSC::JSValue::encode(error);
}

JSC::JSValue WebCore::toJS(JSC::JSGlobalObject* globalObject, CommonAbortReason abortReason)
JSC::JSValue WebCore::toJS(JSC::JSGlobalObject* globalObject, CommonAbortReason abortReason, JSC::JSObject* capturedStack)
{
auto& vm = globalObject->vm();
switch (abortReason) {
case CommonAbortReason::Timeout: {
return createDOMException(globalObject, ExceptionCode::TimeoutError, "The operation timed out."_s);
auto error = createDOMException(globalObject, ExceptionCode::TimeoutError, "The operation timed out."_s);

// If we have a captured stack (from AbortSignal.timeout() call site), copy it to this error
if (capturedStack) {
auto scope = DECLARE_CATCH_SCOPE(vm);
JSValue stackValue = capturedStack->get(globalObject, vm.propertyNames->stack);
if (!scope.exception() && stackValue && !stackValue.isUndefined()) {
if (auto* errorObject = error.getObject()) {
errorObject->putDirect(vm, vm.propertyNames->stack, stackValue, 0);
}
}
scope.clearException();
}
return error;
}
case CommonAbortReason::UserAbort: {
return createDOMException(globalObject, ExceptionCode::AbortError, "The operation was aborted."_s);
Expand All @@ -1664,7 +1678,7 @@ JSC::JSValue WebCore::toJS(JSC::JSGlobalObject* globalObject, CommonAbortReason

extern "C" JSC::EncodedJSValue WebCore__CommonAbortReason__toJS(JSC::JSGlobalObject* globalObject, WebCore::CommonAbortReason abortReason)
{
return JSC::JSValue::encode(WebCore::toJS(globalObject, abortReason));
return JSC::JSValue::encode(WebCore::toJS(globalObject, abortReason, nullptr));
}

JSC::JSObject* Bun::createInvalidThisError(JSC::JSGlobalObject* globalObject, const String& message)
Expand Down
17 changes: 16 additions & 1 deletion src/bun.js/bindings/webcore/AbortSignal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,18 @@ Ref<AbortSignal> AbortSignal::abort(JSDOMGlobalObject& globalObject, ScriptExecu
Ref<AbortSignal> AbortSignal::timeout(ScriptExecutionContext& context, uint64_t milliseconds)
{
auto signal = adoptRef(*new AbortSignal(&context));

// Capture stack trace at creation time for later use when timeout fires
// This ensures the TimeoutError has a stack trace pointing to where timeout() was called
if (auto* globalObject = context.jsGlobalObject()) {
auto& vm = globalObject->vm();
JSC::JSObject* errorForStack = JSC::constructEmptyObject(globalObject);
if (auto* errorInstance = JSC::jsDynamicCast<JSC::ErrorInstance*>(JSC::createError(globalObject, String()))) {
errorInstance->captureStackTrace(vm, globalObject, 1);
signal->m_timeoutCreationStack.setWeakly(errorInstance);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

signal->m_timeout = AbortSignal__Timeout__create(bunVM(context.vm()), signal.ptr(), milliseconds);
ASSERT(signal->m_timeout);
signal->ref();
Expand Down Expand Up @@ -210,7 +222,10 @@ void AbortSignal::signalAbort(JSC::JSGlobalObject* globalObject, CommonAbortReas
return;

m_commonReason = reason;
signalAbort(toJS(globalObject, reason));

// Pass the captured stack trace if this is a timeout signal
JSValue capturedStack = m_timeoutCreationStack.getValue();
signalAbort(toJS(globalObject, reason, capturedStack.isObject() ? jsCast<JSC::JSObject*>(capturedStack) : nullptr));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

void AbortSignal::cleanNativeBindings(void* ref)
Expand Down
4 changes: 3 additions & 1 deletion src/bun.js/bindings/webcore/AbortSignal.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ enum class CommonAbortReason : uint8_t {
ConnectionClosed,
};

JSC::JSValue toJS(JSC::JSGlobalObject*, CommonAbortReason);
JSC::JSValue toJS(JSC::JSGlobalObject*, CommonAbortReason, JSC::JSObject* capturedStack = nullptr);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

typedef void* AbortSignalTimeout;

Expand Down Expand Up @@ -125,6 +125,7 @@ class AbortSignal final : public RefCounted<AbortSignal>, public EventTargetWith
size_t memoryCost() const;

AbortSignalTimeout getTimeout() const { return m_timeout; }
const JSValueInWrappedObject& timeoutCreationStack() const { return m_timeoutCreationStack; }

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
private:
enum class Aborted : bool {
Expand Down Expand Up @@ -189,6 +190,7 @@ class AbortSignal final : public RefCounted<AbortSignal>, public EventTargetWith
std::atomic<uint32_t> pendingActivityCount { 0 };
uint32_t m_algorithmIdentifier { 0 };
AbortSignalTimeout m_timeout { nullptr };
JSValueInWrappedObject m_timeoutCreationStack; // Captured stack at AbortSignal.timeout() call site
uint8_t m_flags { 0 };
};

Expand Down
51 changes: 51 additions & 0 deletions test/js/bun/abort-signal-timeout-stack.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { expect, test, describe } from "bun:test";

describe("AbortSignal.timeout stack trace", () => {
test("should include stack trace pointing to where AbortSignal.timeout() was called", async () => {
// This test verifies fix for issue #25182
// The stack trace should point to where AbortSignal.timeout() was called,
// not just where the timeout was triggered internally

async function myFunction() {
const signal = AbortSignal.timeout(50);
await new Promise((resolve, reject) => {
signal.addEventListener('abort', () => {
reject(signal.reason);
});
setTimeout(resolve, 1000);
});
}
Comment on lines +9 to +17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid setTimeout in tests; rely on the signal or test runner timeout instead

Both tests use setTimeout(resolve, 1000) as a fallback, which goes against the Bun test guidelines to not wait for time to pass in tests; you can safely drop these fallbacks and let the tests either resolve via the abort event or fail via the runner’s own timeout mechanism, reducing flakiness and keeping test timing deterministic.

Also applies to: 41-45

🤖 Prompt for AI Agents
In test/js/bun/abort-signal-timeout-stack.test.ts around lines 9-17 and 41-45,
the tests include a fallback setTimeout(resolve, 1000) which violates the
guideline to avoid waiting in tests; remove the setTimeout fallback from both
places and let the Promise resolve only via the abort event (rejecting with
signal.reason) or time out via the test runner/AbortSignal.timeout so the test
is deterministic and doesn't rely on sleeping.


try {
await myFunction();
expect.unreachable("Should have thrown");
} catch (e: any) {
expect(e instanceof DOMException).toBe(true);
expect(e.name).toBe("TimeoutError");
expect(e.message).toBe("The operation timed out.");

// The stack trace should include 'myFunction'
// since that's where AbortSignal.timeout() was called
const stack = e.stack;
expect(stack).toBeDefined();
expect(typeof stack).toBe("string");

// Check that stack trace includes the calling function
// This is the key fix - previously this would be empty or only have internals
expect(stack).toInclude("myFunction");
}
});

test("stack trace should include the test file name", async () => {
try {
const signal = AbortSignal.timeout(10);
await new Promise((resolve, reject) => {
signal.addEventListener('abort', () => reject(signal.reason));
setTimeout(resolve, 1000);
});
} catch (e: any) {
// Stack should reference this test file
expect(e.stack).toInclude("abort-signal-timeout-stack.test.ts");
}
});
});