Repository navigation
ServerWebSocket: send Blob bytes instead of "[object Blob]" - #36032
Conversation
ServerWebSocket.send(blob), publish(topic, blob), sendBinary(blob), publishBinary(topic, blob), ping(blob), pong(blob), and server.publish(topic, blob) now send the Blob's bytes as a binary frame. Previously send/publish fell through to ToString and shipped the text '[object Blob]' (rc=13) while sendBinary/publishBinary/ping/pong rejected Blobs entirely, so the same Blob meant three different things between the client WebSocket, the server send path, and sendBinary. File- and S3-backed Blobs (Bun.file, Bun.s3) throw a TypeError naming blob.bytes() as the workaround, since these APIs are synchronous and return a byte count. Fixes #5271
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughChangesWebSocket Blob Support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline nits, I also checked the unbound 'a lifetime on blob_payload() — it's sound because as_class_ref::<Blob>() returns &'static Blob, so shared_view() yields a 'static slice; ensure_still_alive() after each send keeps the Blob wrapper (and its Store) live past the borrow. Also verified shared_view() applies offset/size so the Blob.slice() test case is exercising the right code path.
Extended reasoning...
The two inline findings are minor (a test-cleanup edge case and an error-message wording miss). The core change looks correct and well-tested, but it's a user-visible behavior change on the ServerWebSocket send/publish hot path that borrows Blob store bytes across an FFI send — worth a human glance rather than an auto-approve.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Updated 9:23 PM PT - Jul 26th, 2026
✅ @autofix-ci[bot], your commit 89f6a2f63c24d13f4efe4cbee66fd3ecfc4897d2 passed in 🧪 To try this PR locally: bunx bun-pr 36032That installs a local version of the PR into your bun-36032 --bun |
The file-level afterEach stops every server in servers[], so under it.concurrent one sibling finishing would tear down the others' servers mid-flush. Wrap the one fallible await in try/catch instead.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/websocket/websocket-server.test.ts`:
- Line 794: Remove the issue-reference comment adjacent to the WebSocket Blob
transmission test; leave the test logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6718df92-e1e3-4c34-bd33-7579ecd935a3
📒 Files selected for processing (5)
docs/runtime/http/websockets.mdxpackages/bun-types/serve.d.tssrc/runtime/server/ServerWebSocket.rssrc/runtime/server/server_body.rstest/js/bun/websocket/websocket-server.test.ts
There was a problem hiding this comment.
LGTM — all prior review feedback has been addressed.
What was reviewed:
blob_payload()usesas_class_ref::<Blob>()+shared_view()(offset/size-aware, empty for file/S3 stores) withensure_still_alive()after each send, matching the existingjs_string.ensure_still_alive()idiom in this file.- All seven call sites (send/sendBinary/publish/publishBinary/ping/pong/server.publish) are covered by the three new tests, including Blob.slice, File, empty Blob, and file-backed rejection.
openOne()now uses local try/catch cleanup instead of the sharedservers[]array, avoiding theit.concurrent×afterEachrace flagged earlier.
Extended reasoning...
Overview
Adds Blob as an accepted payload type at seven ServerWebSocket send-family sites (send, sendBinary, publish, publishBinary, ping, pong, and server.publish). Previously send/publish fell through to ToString and shipped "[object Blob]"; the binary variants threw. A shared blob_payload() helper detects Blob via as_class_ref::<Blob>(), rejects file-/S3-backed stores with a TypeError pointing at await blob.bytes(), and returns shared_view(). Type defs and docs updated to include Blob in the unions.
Security risks
None. Input is a JS-heap Blob whose bytes are already in memory; no file I/O, no untrusted-length arithmetic (the slice comes straight from shared_view(), which clamps offset/size against the store length). File/S3-backed blobs are rejected before any read.
Level of scrutiny
Medium — native Rust in a hot user-facing send path, but the change is purely additive and mechanical: one new branch inserted between the existing ArrayBuffer check and the string-coercion fallthrough at each site, following the file's exact ensure_still_alive() idiom. as_class_ref returns &'static T kept live by the conservative stack scan while the JSValue is on the stack; the slice is consumed synchronously before ensure_still_alive() roots it past the send. shared_view() at webcore_types.rs:323 already handles offset/size (so blob.slice() works) and returns empty for non-bytes stores.
Other factors
This PR has been through three prior review rounds from me: (1) openOne() server-leak on early reject and the missing "Blob" in the ping/pong TypeError, (2) the servers.push() × it.concurrent × file-level afterEach race that a304bbe introduced, and (3) the "file-backed" wording not covering S3. All are resolved in the current diff (try/catch local cleanup, "string, Blob, or BufferSource", "file- or S3-backed"). CodeRabbit's stray issue-URL nit and comment-cop's long-comment nags were also addressed. The bug hunter found nothing new this run. Test coverage is comprehensive: exact return codes and received bytes asserted for Blob/Blob.slice/File/empty across all seven methods, plus the file-backed error at all seven.
## What
`ServerWebSocket.send(blob)`, `ws.publish(topic, blob)`,
`ws.sendBinary(blob)`, `ws.publishBinary(topic, blob)`, `ws.ping(blob)`,
`ws.pong(blob)`, and `server.publish(topic, blob)` now send the Blob's
bytes as a **binary** frame, matching the client-side
`WebSocket.send(blob)` and npm `ws`.
Previously:
- `send` / `publish` fell through to `ToString` and shipped the literal
text `"[object Blob]"` (return code 13), silently corrupting data.
- `sendBinary` / `publishBinary` / `ping` / `pong` rejected Blobs with a
TypeError.
So passing the same Blob to the client `WebSocket.send`, the server
`ws.send`, and `ws.sendBinary` produced three different results.
## Repro
```js
const server = Bun.serve({
port: 0,
fetch: (req, s) => (s.upgrade(req) ? undefined : new Response("no")),
websocket: { open(ws) { globalThis.sws = ws; ws.subscribe("t"); }, message() {} },
});
const c = new WebSocket(`ws://127.0.0.1:${server.port}/`);
c.binaryType = "arraybuffer";
c.onmessage = e =>
console.log("client got:", typeof e.data === "string" ? JSON.stringify(e.data) : new Uint8Array(e.data));
c.onopen = () => {
const blob = new Blob([new Uint8Array([1, 2, 3, 4])]);
console.log("ws.send(blob) rc =", sws.send(blob)); // before: 13, after: 4
console.log("server.publish rc =", server.publish("t", blob)); // before: 13, after: 4
};
// before: client got: "[object Blob]" "[object Blob]"
// after: client got: Uint8Array [1,2,3,4] Uint8Array [1,2,3,4]
```
## Cause
`send()` / `publish()` check `message_value.as_array_buffer()` and on
miss fall straight through to `message_value.to_js_string()`, which is
the JS `ToString` coercion. Blobs (and every other object) become
`"[object Blob]"`.
## Fix
Insert a `value.as_class_ref::<Blob>()` check between the ArrayBuffer
check and the string coercion at all seven sites, sending
`blob.shared_view()` as `Opcode::Binary`. A shared `blob_payload()`
helper does the detection and rejects file/S3-backed blobs
(`Bun.file()`, `Bun.s3()`) with a TypeError that points at `await
blob.bytes()`, since these send paths are synchronous and return a byte
count so async I/O cannot happen there.
`sendBinary` / `publishBinary` error messages now say "Blob or
BufferSource" instead of "an ArrayBufferView".
Type definitions and the websockets doc page are updated to include
`Blob` in the unions.
## Notes
Non-Blob objects passed to `send` / `publish` still coerce via
`ToString` (e.g. `ws.send(42)` sends `"42"`). Tightening that to a
TypeError the way npm `ws` does would be a separate behavior change.
<!-- robobun:evidence:begin -->
---
**no test proof** · iteration 0 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/websocket/websocket-server.test.ts
<!-- robobun:evidence:end -->
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
What
ServerWebSocket.send(blob),ws.publish(topic, blob),ws.sendBinary(blob),ws.publishBinary(topic, blob),ws.ping(blob),ws.pong(blob), andserver.publish(topic, blob)now send the Blob's bytes as a binary frame, matching the client-sideWebSocket.send(blob)and npmws.Previously:
send/publishfell through toToStringand shipped the literal text"[object Blob]"(return code 13), silently corrupting data.sendBinary/publishBinary/ping/pongrejected Blobs with a TypeError.So passing the same Blob to the client
WebSocket.send, the serverws.send, andws.sendBinaryproduced three different results.Repro
Cause
send()/publish()checkmessage_value.as_array_buffer()and on miss fall straight through tomessage_value.to_js_string(), which is the JSToStringcoercion. Blobs (and every other object) become"[object Blob]".Fix
Insert a
value.as_class_ref::<Blob>()check between the ArrayBuffer check and the string coercion at all seven sites, sendingblob.shared_view()asOpcode::Binary. A sharedblob_payload()helper does the detection and rejects file/S3-backed blobs (Bun.file(),Bun.s3()) with a TypeError that points atawait blob.bytes(), since these send paths are synchronous and return a byte count so async I/O cannot happen there.sendBinary/publishBinaryerror messages now say "Blob or BufferSource" instead of "an ArrayBufferView".Type definitions and the websockets doc page are updated to include
Blobin the unions.Notes
Non-Blob objects passed to
send/publishstill coerce viaToString(e.g.ws.send(42)sends"42"). Tightening that to a TypeError the way npmwsdoes would be a separate behavior change.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/websocket/websocket-server.test.ts