webcore: propagate exceptions from readableStreamTee instead of reporting them as uncaught

Merged
RO
4mo ago
+45-6

Problem

Calling .clone() on a Request or Response whose body stream is locked is supposed to throw a single, catchable TypeError. In ASSERT_ENABLED builds (debug, and the asan release lane) it instead did three wrong things at once:

const q = new Request("http://example.com/", {
  method: "POST",
  body: new ReadableStream({ start() {} }),
  duplex: "half",
});
q.body.getReader(); // lock the body stream
try {
  q.clone();
} catch (e) {
  console.log("caught:", e.message);
}

TypeError: ReadableStream is locked   <- reported as UNCAUGHT even though it was caught
      at repro.ts:8:5
caught: Value is not a sequence       <- the catch block gets the wrong error
(exit code 1)                         <- process fails despite the try/catch

Release builds behaved correctly (caught: ReadableStream is locked, exit 0). Response.clone() hits the same path.

Cause

ReadableStream__tee invokes the readableStreamTee builtin through a local helper in src/jsc/bindings/webcore/ReadableStream.cpp. That builtin throws a TypeError("ReadableStream is locked") synchronously when the stream is already locked, which is exactly what Request.clone() relies on.

The helper contained this after the call:

#if ASSERT_ENABLED
        if (scope.exception()) [[unlikely]] {
            Bun__reportError(lexicalGlobalObject, JSC::JSValue::encode(scope.exception()));
        }
#endif
        EXCEPTION_ASSERT(!scope.exception() || vm.hasPendingTerminationException());
        RETURN_IF_EXCEPTION(scope, {});

Bun__reportError routes through VirtualMachine::uncaught_exception, which prints the error, sets the process exit code to 1, and clears the pending exception. With the exception gone, RETURN_IF_EXCEPTION no longer fired, so control reached convert<IDLSequence<...>> on the empty JSValue returned by the failed JSC::call, which is where the bogus "Value is not a sequence" came from.

The EXCEPTION_ASSERT on the next line encodes the same false invariant (that readableStreamTee can never throw a non-termination exception); without the Bun__reportError clearing the exception it would have aborted instead.

Fix

Drop both lines and let RETURN_IF_EXCEPTION propagate the exception. ReadableStream__tee already returns false with the exception pending, and the Rust caller (from_js_host_call_generic in ReadableStream::tee) already converts that into an Err that reaches the user's catch. This makes debug and asan builds match what release builds already did.

ReadableStream.prototype.tee() was never affected: it goes through the JS builtin directly and does not use this C++ wrapper.

Verification

New test.each(["Request", "Response"]) cases in test/js/web/fetch/body-clone.test.ts spawn a child, lock the body, clone(), and assert the exact caught message and exit code 0.

Before (debug build):

  {
-   "exitCode": 0,
+   "exitCode": 1,
    "stdout": [
-     "caught TypeError: ReadableStream is locked",
+     "caught TypeError: Value is not a sequence",
      "done",
    ],
  }

After: both pass, and the existing body-clone.test.ts (27), body.test.ts / body-stream.test.ts / body-mixin-errors.test.ts (9434), and streams.test.js suites still pass.