fix: Bun.write with new Response(req.body) no longer hangs
#28112
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| // https://github.com/oven-sh/bun/issues/13237 | ||
| // | ||
| // Bun.write with a Response/Request whose body is a ReadableStream used to | ||
| // hang forever: the `.Locked` body path only installed an `onReceiveValue` | ||
| // callback and never actually read from the stream, so the returned promise | ||
| // never settled. | ||
| // | ||
| // These tests spawn subprocesses so a hang turns into a clean failure instead | ||
| // of blocking the test runner. | ||
|
|
||
| import { describe, expect, it } from "bun:test"; | ||
| import { bunEnv, bunExe, tempDir } from "harness"; | ||
| import { join } from "path"; | ||
|
|
||
| async function run(dir: string, script: string) { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", script], | ||
| env: bunEnv, | ||
| cwd: dir, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| timeout: 10_000, | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| return { stdout, stderr, exitCode }; | ||
| } | ||
|
|
||
| describe("Bun.write with a ReadableStream-backed body", () => { | ||
| it("Bun.write(path, new Response(ReadableStream))", async () => { | ||
| using dir = tempDir("issue-13237-response-stream", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const stream = new ReadableStream({ | ||
| start(controller) { | ||
| controller.enqueue(new TextEncoder().encode("hello ")); | ||
| controller.enqueue(new TextEncoder().encode("world")); | ||
| controller.close(); | ||
| }, | ||
| }); | ||
| const written = await Bun.write(${JSON.stringify(out)}, new Response(stream)); | ||
| console.log(JSON.stringify({ written, text: await Bun.file(${JSON.stringify(out)}).text() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ written: 11, text: "hello world" }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("Bun.write(path, new Request(url, { body: ReadableStream }))", async () => { | ||
| using dir = tempDir("issue-13237-request-stream", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const stream = new ReadableStream({ | ||
| start(controller) { | ||
| controller.enqueue(new TextEncoder().encode("from request")); | ||
| controller.close(); | ||
| }, | ||
| }); | ||
| const req = new Request("http://example.com", { method: "POST", body: stream }); | ||
| const written = await Bun.write(${JSON.stringify(out)}, req); | ||
| console.log(JSON.stringify({ written, text: await Bun.file(${JSON.stringify(out)}).text() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ written: 12, text: "from request" }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("Bun.file(path).write(new Response(ReadableStream))", async () => { | ||
| using dir = tempDir("issue-13237-file-write", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const stream = new ReadableStream({ | ||
| start(controller) { | ||
| controller.enqueue(new TextEncoder().encode("instance method")); | ||
| controller.close(); | ||
| }, | ||
| }); | ||
| const written = await Bun.file(${JSON.stringify(out)}).write(new Response(stream)); | ||
| console.log(JSON.stringify({ written, text: await Bun.file(${JSON.stringify(out)}).text() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ written: 15, text: "instance method" }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("Bun.write(path, new Response(req.body)) inside a server handler", async () => { | ||
| using dir = tempDir("issue-13237-server-response-body", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const server = Bun.serve({ | ||
| port: 0, | ||
| async fetch(req) { | ||
| const written = await Bun.write(${JSON.stringify(out)}, new Response(req.body)); | ||
| return new Response(String(written)); | ||
| }, | ||
| }); | ||
| const res = await fetch(server.url, { method: "POST", body: "hello from server" }); | ||
| const written = await res.text(); | ||
| server.stop(); | ||
| console.log(JSON.stringify({ written, text: await Bun.file(${JSON.stringify(out)}).text() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ written: "17", text: "hello from server" }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("Bun.write(path, new Response(ReadableStream)) truncates the destination file", async () => { | ||
| using dir = tempDir("issue-13237-truncate", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const body = (n, c) => | ||
| new Response( | ||
| new ReadableStream({ | ||
| start(controller) { | ||
| controller.enqueue(Buffer.alloc(n, c)); | ||
| controller.close(); | ||
| }, | ||
| }), | ||
| ); | ||
| await Bun.write(${JSON.stringify(out)}, body(1000, "A")); | ||
| await Bun.write(${JSON.stringify(out)}, body(100, "B")); | ||
| const text = await Bun.file(${JSON.stringify(out)}).text(); | ||
| console.log(JSON.stringify({ length: text.length, ok: text === Buffer.alloc(100, "B").toString() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ length: 100, ok: true }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("Bun.write(path, req) after accessing req.body inside a server handler", async () => { | ||
| using dir = tempDir("issue-13237-server-body-access", {}); | ||
| const out = join(String(dir), "out.txt"); | ||
| const { stdout, stderr, exitCode } = await run( | ||
| String(dir), | ||
| ` | ||
| const server = Bun.serve({ | ||
| port: 0, | ||
| async fetch(req) { | ||
| if (!req.body) return new Response("no body", { status: 400 }); | ||
| const written = await Bun.write(${JSON.stringify(out)}, req); | ||
| return new Response(String(written)); | ||
| }, | ||
| }); | ||
| const res = await fetch(server.url, { method: "POST", body: "body was touched" }); | ||
| const written = await res.text(); | ||
| server.stop(); | ||
| console.log(JSON.stringify({ written, text: await Bun.file(${JSON.stringify(out)}).text() })); | ||
| `, | ||
| ); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ written: "16", text: "body was touched" }); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Honoring
truncatehere is correct forpipeReadableStreamToBlob, butflags()is also reached fromBun.file(path).writer()on POSIX (Blob.zig:3008-3019 →sink.start()→setup()→openForWriting(..., options.flags(), ...)), so that API now opens withO_TRUNCwhere it previously did not. The WindowsgetWriterbranch (Blob.zig:~2935) still opens withWRONLY | CREAT | NONBLOCKand bypassesflags(), so this introduces a POSIX-vs-Windows divergence forBun.file().writer()— worth either addingO_TRUNCthere too or calling this out as an intentional behavior change.Extended reasoning...
What changed and why it has wider reach
This PR changes
FileSink.Options.flags()from dead code (_ = this; return NONBLOCK|CLOEXEC|CREAT|WRONLY) to actually honoring thetruncate: bool = truefield by addingO_TRUNCwhen set. The intent is to fixpipeReadableStreamToBlobso that overwriting a file with a shorter ReadableStream-backed Response doesn't leave stale trailing bytes.However,
flags()is not only used bypipeReadableStreamToBlob. It is also reached fromFileSink.setup()(line 312:bun.io.openForWriting(..., options.flags(), ...)), andsetup()is called fromFileSink.start()for anystreams.Start{.FileSink = ...}. The public APIBun.file(path).writer()on POSIX takes exactly this path.The affected call chain
On non-Windows platforms,
Blob.getWriter(Blob.zig:3008-3019) constructs:This leaves
truncateat its struct default oftrue. It then callssink.start(stream_start)→FileSink.start()→FileSink.setup()→bun.io.openForWriting(bun.FD.cwd(), options.input_path, options.flags(), ...).openForWritingImpl(src/io/openForWriting.zig:60) passesinput_flagsdirectly tobun.sys.openatA(dir, path, input_flags, mode)for the.pathcase, so the newO_TRUNCreaches the actualopen()syscall.The same applies to
streams.Start.fromJSWithTagfor the.FileSinkcase (streams.zig:117-173): it readshighWaterMark/path/fdfrom JS but never reads atruncateoption, so it stays at the defaulttrue. There is no JS-side way to opt out.The cross-platform divergence this introduces
On Windows,
Blob.getWritertakes a completely separate branch (Blob.zig:~2928-2989). For path-backed blobs it opens directly via:This bypasses
FileSink.Options.flags()entirely and does not includeO_TRUNC. So before this PR, neither platform truncated; after this PR, POSIX truncates but Windows does not. The PR addedO_TRUNCto the Windows open inpipeReadableStreamToBlob(Blob.zig:2665) but not to the Windows open ingetWriter.Step-by-step proof
/tmp/x.txt(e.g.Bun.write('/tmp/x.txt', '0123456789')).const w = Bun.file('/tmp/x.txt').writer(); w.write('abc'); await w.end();getWriter(POSIX branch) buildsstreams.Start{.FileSink = .{.input_path = .{.path = '/tmp/x.txt'}}}withtruncatedefaulting totrue.sink.start()→setup()→openForWriting(..., NONBLOCK|CLOEXEC|CREAT|WRONLY|TRUNC, ...)→ file is truncated to 0 bytes on open.end(), the file containsabc(3 bytes). Before this PR, the open usedNONBLOCK|CLOEXEC|CREAT|WRONLY(no TRUNC), so the file would containabc3456789(10 bytes).getWriteropens withWRONLY|CREAT|NONBLOCK(no TRUNC, line ~2935), so the file still containsabc3456789.Result: POSIX and Windows now disagree on what
Bun.file(path).writer()does to existing file contents.Why existing code doesn't prevent it
The
truncate: bool = truefield has always existed with that default —_ = this;was clearly incomplete dead code, so honoring it is arguably the long-intended behavior. CI is green because all existing FileSink/writer tests write to fresh temp files and never overwrite. But this is still an undocumented behavior change to a public API surface (Bun.file().writer()) that is unrelated to theBun.write-hangs-on-locked-body bug this PR fixes, and the new cross-platform inconsistency is a concrete defect introduced here.How to fix
The cleanest minimal fix is to add
bun.O.TRUNCto the WindowsgetWriteropen at Blob.zig:~2935 so both platforms agree (and match the field's stated default). Alternatively, if you want to avoid touchingBun.file().writer()semantics in this PR, you could revertflags()and instead pass an explicit truncating flag set only frompipeReadableStreamToBlob's POSIX path — but since truncate-by-default is almost certainly what users expect from a file writer, aligning Windows is probably the right call.