Skip to content

feat: support binary WebSocket frames in message handlers#1334

Open
sansyrox wants to merge 1 commit intomainfrom
feat/binary-websocket-frames
Open

feat: support binary WebSocket frames in message handlers#1334
sansyrox wants to merge 1 commit intomainfrom
feat/binary-websocket-frames

Conversation

@sansyrox
Copy link
Copy Markdown
Member

@sansyrox sansyrox commented Mar 21, 2026

Summary

Closes #1332

  • Adds a WsPayload enum (Text(String) / Binary(Vec<u8>)) that replaces String throughout the entire WebSocket pipeline: channel, registry messages, send/broadcast methods
  • Binary frames (ws::Message::Binary) are now forwarded through the Rust channel to Python instead of being silently echoed back
  • channel.receive() returns str for text frames and bytes for binary frames
  • send_to / broadcast methods accept both str and bytes, emitting the correct WebSocket frame type (ctx.text() / ctx.binary())
  • Python WebSocketAdapter gains a new receive() method returning str | bytes, plus receive_text() / receive_bytes() now validate frame type instead of faking conversion

Usage

@app.websocket("/ws")
async def handler(ws):
    while True:
        msg = await ws.receive()  # str | bytes
        if isinstance(msg, bytes):
            audio = pcm16_to_float32(msg)  # binary frame — zero-copy path
        else:
            control = orjson.loads(msg)     # text frame — JSON control signals

        await ws.send_bytes(b"\x00\x01\x02")       # binary frame
        await ws.send_text('{"type": "done"}')      # text frame

Files changed

File What changed
src/websockets/mod.rs WsPayload enum, channel type Option<String>Option<WsPayload>, StreamHandler forwards binary, send methods accept &Bound<PyAny>
src/websockets/registry.rs SendTextSendMessage with payload: WsPayload, SendMessageToAll likewise
src/executors/web_socket_executors.rs Return value extraction handles both str and bytes, dispatches to ctx.text() / ctx.binary()
robyn/ws.py receive()str | bytes, real receive_text() / receive_bytes() with type validation, send_bytes() sends actual binary frames

Test plan

  • Verify text-only WebSocket handlers still work unchanged
  • Connect a WebSocket client that sends binary frames, verify Python handler receives bytes
  • Send binary frames from Python via send_bytes(), verify client receives binary opcode
  • Test receive_text() raises TypeError when binary frame arrives
  • Test receive_bytes() raises TypeError when text frame arrives
  • Test broadcast() with both str and bytes
  • Run existing integration test suite

Made with Cursor

Summary by CodeRabbit

Release Notes

  • New Features

    • Added native binary message frame support for WebSocket connections
    • New receive() method returns the next frame as its native type (str or bytes)
    • New send() method sends both text and binary payloads
    • broadcast() now accepts both str and bytes payloads
  • Documentation

    • Updated API reference with binary messaging examples
    • Documented type-checking behavior for receive_text() and receive_bytes()

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
robyn Ready Ready Preview, Comment Mar 28, 2026 7:26pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 21, 2026

📝 Walkthrough

Walkthrough

This PR adds native binary WebSocket frame support throughout the stack. The Python adapter's core API shifts from text-only receive_text() to a unified receive() → str | bytes that preserves frame types. New send(data: str | bytes) sends either type directly. Internal message queues and handler extraction logic updated to carry WsPayload enum variants instead of forcing all frames to strings.

Changes

Cohort / File(s) Summary
Core WebSocket Adapter
robyn/ws.py
Replaced receive_text() core with new `receive() → str
Handler Extraction Logic
src/executors/web_socket_executors.rs
Introduced extract_ws_return() helper that converts Python handler results (None, str, Vec<u8>) to Option<WsPayload>. Updated async and sync handler completion paths to match on WsPayload variants and dispatch via ctx.text() or ctx.binary().
Message Queue & Frame Processing
src/websockets/mod.rs
Added WsPayload enum (Text(String), Binary(Vec<u8>)). Replaced Python message queue from Option<String> to Option<WsPayload>. Updated send_to and broadcast Python APIs to accept and validate `str
Message Routing
src/websockets/registry.rs
Renamed SendText to SendMessage carrying WsPayload instead of message: String. Updated broadcast to use WsPayload variants. Modified close notification to send WsPayload::Text("Connection closed").
Type Hints
robyn/robyn.pyi
Updated WebSocketConnector method signatures: async_broadcast(), async_send_to(), sync_broadcast(), sync_send_to() now accept `message: str
Documentation
docs_src/src/pages/documentation/en/api_reference/websockets.mdx, docs_src/src/pages/documentation/zh/api_reference/websockets.mdx
Documented new receive() method returning `str

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Actix as Actix-web<br/>(WebSocket)
    participant Rust as Rust Layer<br/>(mod.rs)
    participant Connector as WebSocketConnector<br/>(Actor)
    participant PyQueue as Python<br/>Message Queue
    participant Handler as Python Handler
    participant Adapter as WebSocketAdapter

    rect rgba(100, 150, 200, 0.5)
    Note over Client,Adapter: Binary Frame Reception Flow (New)
    Client->>Actix: Send binary frame
    Actix->>Rust: ws::Message::Binary(bytes)
    Rust->>Rust: Wrap as WsPayload::Binary
    Rust->>PyQueue: Enqueue WsPayload::Binary
    Rust->>Connector: (inbound complete)
    end

    rect rgba(100, 150, 200, 0.5)
    Note over Client,Adapter: Handler Invocation & Dispatch
    Handler->>Adapter: call receive()
    Adapter->>PyQueue: dequeue WsPayload
    PyQueue->>Adapter: return WsPayload::Binary(bytes)
    Adapter->>Handler: return bytes
    Handler->>Handler: isinstance(msg, bytes) → process audio
    Handler->>Adapter: call send(binary_data)
    end

    rect rgba(100, 150, 200, 0.5)
    Note over Client,Adapter: Binary Frame Transmission Flow (New)
    Adapter->>Rust: send(bytes)
    Rust->>Connector: SendMessage{payload: WsPayload::Binary}
    Connector->>Actix: Handler processes SendMessage
    Actix->>Actix: ctx.binary(bytes)
    Actix->>Client: Send binary frame
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐇 hops with joy
Binary frames hop through, text frames too!
No base64 chains when bytes come through,
PCM whispers, clean and light—
Frames preserved in their true type! ✨🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective: adding support for binary WebSocket frames in message handlers, which aligns with the core feature across all modified files.
Description check ✅ Passed The PR description includes a comprehensive summary of changes, motivating usage examples, a files-changed table, and a test plan covering the key requirements.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #1332 objectives: preserves Text/Binary frame distinction [#1332], delivers payloads as str/bytes [#1332], enables send APIs to accept both types [#1332], and supports zero-copy binary handling [#1332].
Out of Scope Changes check ✅ Passed All changes are scoped to implementing binary WebSocket frame support: WsPayload enum in Rust layer, updated channel types, send/receive APIs, Python adapter methods, documentation updates, and type stubs. No unrelated functionality is introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/binary-websocket-frames

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/executors/web_socket_executors.rs (1)

9-20: Silent fallback to None for unsupported return types may mask handler bugs.

When a Python handler returns an unsupported type (e.g., int, dict, list), extract_ws_return silently returns None, resulting in no WebSocket frame being sent. This could mask programming errors in user handlers where they accidentally return an unexpected type.

Consider logging a warning for the fallback case to aid debugging:

🔧 Proposed enhancement to add a debug warning
 fn extract_ws_return(_py: Python, output: &Bound<'_, PyAny>) -> Option<WsPayload> {
     if output.is_none() {
         return None;
     }
     if let Ok(s) = output.extract::<String>() {
         Some(WsPayload::Text(s))
     } else if let Ok(b) = output.extract::<Vec<u8>>() {
         Some(WsPayload::Binary(b))
     } else {
+        log::debug!(
+            "WebSocket handler returned unsupported type '{}'; no frame sent",
+            output.get_type().name().unwrap_or_default()
+        );
         None
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/executors/web_socket_executors.rs` around lines 9 - 20, extract_ws_return
currently silently returns None for unsupported Python return types, which can
hide handler bugs; update extract_ws_return to log a warning (including the
Python object's type and/or repr) when neither String nor Vec<u8> can be
extracted so developers can see unexpected return values from their handlers.
Locate the extract_ws_return function and add a warning/emitter (e.g., using the
existing logger/tracing facility or a new warn! call) inside the final else
branch that reports the Python object's type and a brief repr, then still return
None to preserve behavior. Ensure the log text references extract_ws_return and
the handler context to make debugging straightforward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@robyn/ws.py`:
- Around line 68-86: Update the type stubs in robyn.pyi so the websocket
send/broadcast methods accept both str and bytes: change the parameter type of
async_broadcast, async_send_to, sync_broadcast, and sync_send_to from just str
to str | bytes (or Union[str, bytes] if using typing.Union in stubs), ensuring
you import Union if needed and keep existing return types and overloads intact
so the stubs match the implementations used by ws.py (methods async_send_to,
async_broadcast, send_text, send_bytes, send_json, broadcast).

In `@src/websockets/mod.rs`:
- Around line 215-239: The async_send_to method uses Uuid::parse_str(...) and
currently calls unwrap(), which can panic on invalid UUID input; replace that
unwrap with safe parsing that returns a Python error instead of panicking:
validate the recipient_id by matching Uuid::parse_str(&recipient_id) (or using
map_err) and on Err convert the uuid parse error into a PyErr (e.g.,
PyErr::new::<pyo3::exceptions::PyValueError, _>(...)) and return Err(PyErr) from
async_send_to; keep the rest of the function (payload extraction,
registry.try_send, sender_id) unchanged so the method returns a PyResult without
panics.
- Around line 200-212: In sync_send_to, avoid panicking by replacing
Uuid::parse_str(&recipient_id).unwrap() with proper error handling: call
Uuid::parse_str(&recipient_id).map_err(|e| PyErr::new_err(format!("Invalid
recipient_id UUID: {}", e)))? to return a Python error instead of panicking;
keep the parsed Uuid in the same recipient_id variable and proceed with the
existing match that sends SendMessage via self.registry_addr.try_send. Ensure
the function returns Err(PyErr) on parse failure so malformed UUIDs are reported
to Python rather than crashing the process.

---

Nitpick comments:
In `@src/executors/web_socket_executors.rs`:
- Around line 9-20: extract_ws_return currently silently returns None for
unsupported Python return types, which can hide handler bugs; update
extract_ws_return to log a warning (including the Python object's type and/or
repr) when neither String nor Vec<u8> can be extracted so developers can see
unexpected return values from their handlers. Locate the extract_ws_return
function and add a warning/emitter (e.g., using the existing logger/tracing
facility or a new warn! call) inside the final else branch that reports the
Python object's type and a brief repr, then still return None to preserve
behavior. Ensure the log text references extract_ws_return and the handler
context to make debugging straightforward.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bdd2cff9-cfa1-47cd-90c4-e77f296c3e4c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c3be94 and cc3d532.

📒 Files selected for processing (4)
  • robyn/ws.py
  • src/executors/web_socket_executors.rs
  • src/websockets/mod.rs
  • src/websockets/registry.rs

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 21, 2026

Merging this PR will not alter performance

✅ 189 untouched benchmarks


Comparing feat/binary-websocket-frames (4594e66) with main (4ea10a7)

Open in CodSpeed

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/websockets/mod.rs`:
- Around line 28-33: The WsPayload enum's Binary variant currently uses Vec<u8>,
forcing owned copies through the bridge and causing extra clones in the
broadcast path; change WsPayload::Binary to use a zero-copy buffer type (e.g.,
bytes::Bytes or Arc<[u8]>) and update the inbound frame handling (the code
around the binary frame copy at line ~193) to construct that zero-copy type
instead of allocating a Vec, and then update the broadcast/registry logic in
registry.rs (the code that currently clones the payload per recipient at lines
~87-97) to pass or cheaply clone the zero-copy buffer (Bytes::clone or
Arc::clone) so broadcasts no longer perform full buffer copies. Ensure type
changes propagate to any match arms or function signatures that pattern-match on
WsPayload::Binary.
- Line 99: The code currently creates a bounded channel (let (tx, rx) =
mpsc::channel::<Option<WsPayload>>(256)) but elsewhere uses non-blocking sends
that silently drop frames when the buffer is full (lines around the
try_send/drop logic at the send site), which loses payloads; change the
send-side to preserve backpressure or fail fast: replace non-blocking
try_send/drop logic with an awaitable send (tx.send(Some(payload)).await) so the
task pauses until space is available, or if you prefer fail-fast, detect the
full/closed condition and close the websocket/return an error (handle
mpsc::error::SendError) rather than discarding frames; use the tx, rx and
WsPayload symbols to locate the sender/receiver and update send handling and
error/close logic accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8c40f335-5016-46b6-bd94-1f3ff7f9f52f

📥 Commits

Reviewing files that changed from the base of the PR and between f8d2452 and f5dd683.

📒 Files selected for processing (8)
  • .github/workflows/release-CI.yml
  • docs_src/src/pages/documentation/en/api_reference/websockets.mdx
  • docs_src/src/pages/documentation/zh/api_reference/websockets.mdx
  • robyn/robyn.pyi
  • robyn/ws.py
  • src/executors/web_socket_executors.rs
  • src/websockets/mod.rs
  • src/websockets/registry.rs
✅ Files skipped from review due to trivial changes (1)
  • docs_src/src/pages/documentation/en/api_reference/websockets.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/release-CI.yml
  • src/websockets/registry.rs
  • robyn/ws.py

Comment on lines +28 to +33
#[derive(Clone)]
pub enum WsPayload {
Text(String),
Binary(Vec<u8>),
Close,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Vec<u8> prevents the new binary path from being zero-copy.

WsPayload::Binary(Vec<u8>) forces owned buffers through the bridge. Line 193 already copies each inbound binary frame into a fresh Vec, and src/websockets/registry.rs Lines 87-97 clone that payload again for each broadcast recipient, so the zero-copy part of #1332 still is not met for audio/streaming workloads.

Also applies to: 190-193

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` around lines 28 - 33, The WsPayload enum's Binary
variant currently uses Vec<u8>, forcing owned copies through the bridge and
causing extra clones in the broadcast path; change WsPayload::Binary to use a
zero-copy buffer type (e.g., bytes::Bytes or Arc<[u8]>) and update the inbound
frame handling (the code around the binary frame copy at line ~193) to construct
that zero-copy type instead of allocating a Vec, and then update the
broadcast/registry logic in registry.rs (the code that currently clones the
payload per recipient at lines ~87-97) to pass or cheaply clone the zero-copy
buffer (Bytes::clone or Arc::clone) so broadcasts no longer perform full buffer
copies. Ensure type changes propagate to any match arms or function signatures
that pattern-match on WsPayload::Binary.

});

let (tx, rx) = mpsc::unbounded_channel::<Option<String>>();
let (tx, rx) = mpsc::channel::<Option<WsPayload>>(256);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't turn backpressure into silent frame loss.

Line 99 caps the Python-side queue at 256 entries, and Lines 185-194 drop frames once it fills. Slow consumers will start missing text/binary messages with no signal to the handler, which is especially dangerous for ordered binary streams like audio. Please preserve queueing semantics or close on overload instead of discarding payloads.

Also applies to: 184-195

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` at line 99, The code currently creates a bounded
channel (let (tx, rx) = mpsc::channel::<Option<WsPayload>>(256)) but elsewhere
uses non-blocking sends that silently drop frames when the buffer is full (lines
around the try_send/drop logic at the send site), which loses payloads; change
the send-side to preserve backpressure or fail fast: replace non-blocking
try_send/drop logic with an awaitable send (tx.send(Some(payload)).await) so the
task pauses until space is available, or if you prefer fail-fast, detect the
full/closed condition and close the websocket/return an error (handle
mpsc::error::SendError) rather than discarding frames; use the tx, rx and
WsPayload symbols to locate the sender/receiver and update send handling and
error/close logic accordingly.

- Add WsPayload enum (Text/Binary) to replace String-only messages
- Update WebSocketChannel.receive() to return str for text, bytes for binary
- Add receive(), receive_bytes(), send(), send_bytes() to WebSocketAdapter
- Update type stubs to accept str | bytes for send/broadcast methods
- Replace Uuid::parse_str().unwrap() with proper PyValueError handling
- Update broadcast() to accept str | bytes for binary frame support
- Update WebSocket docs with binary frame examples

Made-with: Cursor
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/websockets/mod.rs (2)

98-98: Unbounded channel trades backpressure for memory risk.

Switching to unbounded_channel eliminates the silent frame loss flagged in a previous review, but a slow Python consumer can now cause unbounded memory growth. For most WebSocket workloads this is acceptable, but consider documenting this behavior or adding optional high-water-mark warnings for streaming use cases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` at line 98, The current use of
mpsc::unbounded_channel::<Option<WsPayload>>() (creating tx and rx) removes
earlier silent drops but opens the door to unbounded memory growth if the Python
consumer is slow; either document this tradeoff in websockets/mod.rs and the
public API, or replace the unbounded channel with a bounded
mpsc::channel(capacity) and handle full-channel backpressure (e.g., propagate
errors, await send, or drop with a metric), and optionally add a high-water-mark
check tied to tx/queue length that emits warnings via the existing logger when a
threshold is exceeded to alert on streaming backpressure.

154-163: Extract "Connection closed" sentinel to a shared constant.

The magic string "Connection closed" is duplicated here and in registry.rs (line 115). A typo in either location would silently break close detection.

♻️ Proposed refactor

Add to a shared location (e.g., src/websockets/mod.rs or a constants module):

pub const WS_CLOSE_SENTINEL: &str = "Connection closed";

Then update both files:

 // In mod.rs Handler<SendMessage>
 WsPayload::Text(s) => {
     ctx.text(s.clone());
-    if s == "Connection closed" {
+    if s == WS_CLOSE_SENTINEL {
         ctx.stop();
     }
 }
 // In registry.rs Handler<Close>
 client.do_send(SendMessage {
     recipient_id: msg.id,
-    payload: WsPayload::Text("Connection closed".to_string()),
+    payload: WsPayload::Text(WS_CLOSE_SENTINEL.to_string()),
     sender_id: msg.id,
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` around lines 154 - 163, Introduce a single public
constant (e.g., pub const WS_CLOSE_SENTINEL: &str = "Connection closed";) in the
websockets module root and replace all hard-coded occurrences of the literal
with that constant; specifically, in the match over msg.payload (WsPayload::Text
branch) replace the ctx.text(s.clone())/s == "Connection closed" string usage
with the constant (use it for sending and for the equality check), and update
the matching usage in registry.rs to import and compare against the same
WS_CLOSE_SENTINEL so both close detection and emitted text come from one shared
symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/websockets/mod.rs`:
- Around line 240-250: The CI failure is due to rustfmt formatting of the
closure on the registry.try_send(...) call; open the block where
runtime::future_into_py(py, async move { ... }) is created (the code that
constructs SendMessage with payload, sender_id, recipient_id) and run cargo fmt
or reformat so the .map_err(|e| { ... }) closure is placed/indented according to
rustfmt (e.g., move .map_err to its own line and format the closure body),
ensuring runtime::future_into_py, registry.try_send, SendMessage and the map_err
closure are properly aligned.
- Around line 272-278: The closure formatting on the registry.try_send call
within the runtime::future_into_py async block doesn't match rustfmt; reformat
the map_err closure for SendMessageToAll so it aligns with rustfmt style (place
the .map_err on its own line and the closure body on the next line, returning
anyhow::anyhow! with the error), i.e., adjust the awaitable block around
runtime::future_into_py, registry.try_send, SendMessageToAll, and map_err to
follow rustfmt line/indentation conventions.

---

Nitpick comments:
In `@src/websockets/mod.rs`:
- Line 98: The current use of mpsc::unbounded_channel::<Option<WsPayload>>()
(creating tx and rx) removes earlier silent drops but opens the door to
unbounded memory growth if the Python consumer is slow; either document this
tradeoff in websockets/mod.rs and the public API, or replace the unbounded
channel with a bounded mpsc::channel(capacity) and handle full-channel
backpressure (e.g., propagate errors, await send, or drop with a metric), and
optionally add a high-water-mark check tied to tx/queue length that emits
warnings via the existing logger when a threshold is exceeded to alert on
streaming backpressure.
- Around line 154-163: Introduce a single public constant (e.g., pub const
WS_CLOSE_SENTINEL: &str = "Connection closed";) in the websockets module root
and replace all hard-coded occurrences of the literal with that constant;
specifically, in the match over msg.payload (WsPayload::Text branch) replace the
ctx.text(s.clone())/s == "Connection closed" string usage with the constant (use
it for sending and for the equality check), and update the matching usage in
registry.rs to import and compare against the same WS_CLOSE_SENTINEL so both
close detection and emitted text come from one shared symbol.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: df52e6ab-f153-4a80-b22c-4df25b597887

📥 Commits

Reviewing files that changed from the base of the PR and between f5dd683 and 4594e66.

📒 Files selected for processing (7)
  • docs_src/src/pages/documentation/en/api_reference/websockets.mdx
  • docs_src/src/pages/documentation/zh/api_reference/websockets.mdx
  • robyn/robyn.pyi
  • robyn/ws.py
  • src/executors/web_socket_executors.rs
  • src/websockets/mod.rs
  • src/websockets/registry.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/executors/web_socket_executors.rs
  • docs_src/src/pages/documentation/zh/api_reference/websockets.mdx
  • robyn/robyn.pyi
  • src/websockets/registry.rs
  • robyn/ws.py

Comment on lines 240 to 250
let awaitable = runtime::future_into_py(py, async move {
match registry.try_send(SendText {
message,
sender_id,
recipient_id,
}) {
Ok(_) => log::debug!("Message sent successfully"),
Err(e) => log::error!("Failed to send message: {}", e),
}
Ok(())
registry
.try_send(SendMessage {
payload,
sender_id,
recipient_id,
})
.map_err(|e| {
anyhow::anyhow!("Failed to enqueue message to registry: {e}")
})
})?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix formatting to pass CI.

The pipeline fails because cargo fmt --check expects the map_err closure to be formatted differently.

🔧 Proposed fix (run `cargo fmt`)
         let awaitable = runtime::future_into_py(py, async move {
-            registry
-                .try_send(SendMessage {
-                    payload,
-                    sender_id,
-                    recipient_id,
-                })
-                .map_err(|e| {
-                    anyhow::anyhow!("Failed to enqueue message to registry: {e}")
-                })
+            registry.try_send(SendMessage {
+                payload,
+                sender_id,
+                recipient_id,
+            }).map_err(|e| anyhow::anyhow!("Failed to enqueue message to registry: {e}"))
         })?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let awaitable = runtime::future_into_py(py, async move {
match registry.try_send(SendText {
message,
sender_id,
recipient_id,
}) {
Ok(_) => log::debug!("Message sent successfully"),
Err(e) => log::error!("Failed to send message: {}", e),
}
Ok(())
registry
.try_send(SendMessage {
payload,
sender_id,
recipient_id,
})
.map_err(|e| {
anyhow::anyhow!("Failed to enqueue message to registry: {e}")
})
})?;
let awaitable = runtime::future_into_py(py, async move {
registry.try_send(SendMessage {
payload,
sender_id,
recipient_id,
}).map_err(|e| anyhow::anyhow!("Failed to enqueue message to registry: {e}"))
})?;
🧰 Tools
🪛 GitHub Actions: Rust Continuous integration

[error] 244-244: cargo fmt --check failed: formatting differs from rustfmt output (diff shows map_err closure condensed to a single line).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` around lines 240 - 250, The CI failure is due to
rustfmt formatting of the closure on the registry.try_send(...) call; open the
block where runtime::future_into_py(py, async move { ... }) is created (the code
that constructs SendMessage with payload, sender_id, recipient_id) and run cargo
fmt or reformat so the .map_err(|e| { ... }) closure is placed/indented
according to rustfmt (e.g., move .map_err to its own line and format the closure
body), ensuring runtime::future_into_py, registry.try_send, SendMessage and the
map_err closure are properly aligned.

Comment on lines 272 to 278
let awaitable = runtime::future_into_py(py, async move {
match registry.try_send(SendMessageToAll { message, sender_id }) {
Ok(_) => log::debug!("Broadcast sent successfully"),
Err(e) => log::error!("Failed to broadcast message: {}", e),
}
Ok(())
registry
.try_send(SendMessageToAll { payload, sender_id })
.map_err(|e| {
anyhow::anyhow!("Failed to enqueue broadcast to registry: {e}")
})
})?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix formatting to pass CI.

Same formatting issue as async_send_to - the map_err closure needs to conform to rustfmt output.

🔧 Proposed fix (run `cargo fmt`)
         let awaitable = runtime::future_into_py(py, async move {
-            registry
-                .try_send(SendMessageToAll { payload, sender_id })
-                .map_err(|e| {
-                    anyhow::anyhow!("Failed to enqueue broadcast to registry: {e}")
-                })
+            registry.try_send(SendMessageToAll { payload, sender_id })
+                .map_err(|e| anyhow::anyhow!("Failed to enqueue broadcast to registry: {e}"))
         })?;
🧰 Tools
🪛 GitHub Actions: Rust Continuous integration

[error] 272-272: cargo fmt --check failed: formatting differs from rustfmt output (diff shows map_err closure condensed to a single line).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/websockets/mod.rs` around lines 272 - 278, The closure formatting on the
registry.try_send call within the runtime::future_into_py async block doesn't
match rustfmt; reformat the map_err closure for SendMessageToAll so it aligns
with rustfmt style (place the .map_err on its own line and the closure body on
the next line, returning anyhow::anyhow! with the error), i.e., adjust the
awaitable block around runtime::future_into_py, registry.try_send,
SendMessageToAll, and map_err to follow rustfmt line/indentation conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support binary WebSocket frames in message handlers

1 participant