Skip to content

fix(backend): break outer loop on client write failure for accesses/TPS - #116

Merged
iamvukasin merged 1 commit into
monad-developers:mainfrom
ayushsingh82:fix/client-write-task-outer-break
Sep 8, 2026
Merged

fix(backend): break outer loop on client write failure for accesses/TPS#116
iamvukasin merged 1 commit into
monad-developers:mainfrom
ayushsingh82:fix/client-write-task-outer-break

Conversation

@ayushsingh82

@ayushsingh82 ayushsingh82 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

client_write_task in backend/src/lib/server.rs sends three kinds of buffered messages to a WebSocket client each iteration: events_buf, accesses_buf, and tps_buf. When send_message() fails for events_buf, the code correctly breaks the outer loop, terminating the task for that (now-dead) connection.

But for accesses_buf and tps_buf, the send happens inside a for loop over the buffered items, and the break on failure only exits that inner for loop — not the outer task loop:

if !accesses_buf.is_empty() {
    for accesses in std::mem::take(&mut accesses_buf) {
        let server_msg = ServerMessage::TopAccesses(accesses);
        if let Err(e) = send_message(&mut ws_sender, server_msg).await {
            error!("Failed to send accesses to {}: {}", addr, e);
            break; // only exits this `for`, outer `loop` keeps going
        }
    }
}

A send_message failure here means the client's WebSocket write is broken (e.g. disconnected). Once that happens, the task should exit — same intent as the events_buf path right above it — but instead it falls through and keeps looping, repeatedly trying (and failing) to serve a dead connection instead of terminating.

Changes

  • Label the outer loop as 'outer and change the two nested breaks (in the accesses_buf and tps_buf send loops) to break 'outer, so a write failure on any buffer terminates the task consistently.
  • No other behavior changes; the unlabeled breaks elsewhere in the function (broadcast receiver error, events send failure) are unaffected since they weren't nested in a for loop to begin with.

Test plan

  • Reproduced the exact control-flow bug in an isolated Rust snippet (bare break inside a for inside a loop only exits the for) and confirmed the labeled-loop fix resolves it.
  • cargo fmt --check passes on the crate.
  • cargo check / cargo clippy / cargo test — could not run locally on macOS; monad-event-ring's build script requires cmake and Linux hugetlbfs headers not available in this environment (same constraint noted in fix(backend): prevent panic when txn_idx exceeds fixed buffer size #115). Deferred to CI.

Greptile Summary

This PR makes WebSocket client write failures terminate the client task consistently.

  • Labels the outer client-write loop.
  • Exits that loop when sending buffered access or TPS messages fails.
  • Leaves existing event and broadcast-receiver error handling unchanged.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
backend/src/lib/server.rs Labels the client-write loop and exits it when access or TPS WebSocket sends fail.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client write loop] --> B{Buffered message available?}
    B -->|Events| C[Send event message]
    B -->|Accesses| D[Send access message]
    B -->|TPS| E[Send TPS message]
    C --> F{Send succeeded?}
    D --> F
    E --> F
    F -->|Yes| A
    F -->|No| G[Exit outer client task loop]
Loading

Reviews (3): Last reviewed commit: "fix(backend): break outer loop on client..." | Re-trigger Greptile

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

@ayushsingh82 is attempting to deploy a commit to the MF Flagship Team on Vercel.

A member of the Team first needs to authorize it.

@ayushsingh82

Copy link
Copy Markdown
Contributor Author

Gentle follow-up here — mergeable, small fix to break the outer loop on client write failure for accesses/TPS so the run doesn't spin after the client goes away. Let me know if you'd like anything changed.

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
monode Ready Ready Preview Sep 8, 2026 10:21am UTC

Request Review

@iamvukasin

Copy link
Copy Markdown
Contributor

@ayushsingh82 Can you rebase on top of the latest changes on main?

@ayushsingh82
ayushsingh82 force-pushed the fix/client-write-task-outer-break branch from 881b23e to b22223b Compare September 8, 2026 09:34
@ayushsingh82

ayushsingh82 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@iamvukasin done, rebased on the latest main (now on f5ef949). Clean rebase, no conflicts.

@iamvukasin

Copy link
Copy Markdown
Contributor

@ayushsingh82 Can you also use verified signatures for commits?

Motivation:

In client_write_task, a failed send_message() for the accesses_buf or
tps_buf messages only broke the inner `for` loop over the buffered
items, not the outer task loop. This is inconsistent with the
events_buf send right above it, which correctly breaks the outer loop
on failure.

A send failure here means the client is gone (write error on the
WebSocket, e.g. broken pipe after disconnect). Once that happens, the
task should stop - but with the inner break, it fell through to the
next iteration of the outer loop and kept trying (and failing) to
serve the same dead connection indefinitely instead of exiting, unlike
every other failure path in this function.

Modifications:

Label the outer loop and change the two nested breaks to `break
'outer'` so a write failure on any buffer (events, accesses, or TPS)
terminates the task the same way.

Result:

client_write_task now exits promptly on any write failure, regardless
of which buffer triggered it, matching its behavior for the other
error paths in the same function (broadcast receiver errors, events
send failures).
@ayushsingh82
ayushsingh82 force-pushed the fix/client-write-task-outer-break branch from b22223b to e0031ad Compare September 8, 2026 09:41
@ayushsingh82

ayushsingh82 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@iamvukasin done. Force-pushed with an SSH-signed commit (now e0031ad). GitHub shows it as Verified. Same diff, only the signature was added.

@iamvukasin
iamvukasin merged commit de031c2 into monad-developers:main Sep 8, 2026
12 checks passed
@iamvukasin

Copy link
Copy Markdown
Contributor

Thank you for your contribution.

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.

2 participants