Skip to content

Conversation

@kixelated
Copy link
Owner

@kixelated kixelated commented Oct 25, 2025

Summary by CodeRabbit

  • Refactor
    • Optimized internal stream frame handling in the WebTransport implementation for improved efficiency and code maintainability. No changes to public APIs or user-facing behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2025

Walkthrough

The recv_frame function in session.rs was refactored to consolidate fin handling logic. Previously, the code used a two-stage pattern: matching to obtain a mutable reference, followed by separate send-and-teardown blocks. The refactored version now handles logic inline within each match arm. For vacant entries, inbound streams are sent to recv_backend before conditionally inserting the state based on the fin flag. For occupied entries, inbound data is updated directly and entries are removed when fin is true. This consolidation moves fin-based removal from post-match logic into the respective match branches. No public API changes were made.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The PR title "Don't use a newer Rust method" addresses the intent behind the changes but lacks specificity about what was actually modified. The title refers to a real aspect of the changeset—avoiding a newer Rust method (likely insert_entry given the source branch name)—but it doesn't convey the primary change, which is a refactoring of the recv_frame function's state handling logic for stream frames. The title is vague because it doesn't identify which method is being avoided or what concrete refactoring was performed, making it difficult for a teammate scanning the PR history to understand the actual code changes without diving into the details.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch insert_entry

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

@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: 0

🧹 Nitpick comments (1)
web-transport-ws/src/session.rs (1)

156-217: LGTM! Vacant entry handling is correct.

The refactored logic correctly handles new incoming streams:

  • Sends the stream frame to recv_backend before conditionally inserting it
  • Only tracks the state in recv_streams if the stream is not immediately finished (!fin)
  • This prevents memory leaks from tracking already-completed streams

The ordering (send at line 212, then conditional insert at lines 214-216) ensures the recv_frontend receives the stream data via its channel, even when the state isn't persisted for fin frames.

Consider adding a brief comment explaining why the state is only inserted when !fin, for example:

// Only track streams that aren't immediately finished
if !fin {
    e.insert(recv_backend);
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 43382d9 and 764a98d.

📒 Files selected for processing (1)
  • web-transport-ws/src/session.rs (2 hunks)
🔇 Additional comments (1)
web-transport-ws/src/session.rs (1)

218-224: LGTM! Occupied entry handling is correct.

The refactored logic properly handles subsequent frames on existing streams:

  • Sends the stream frame to the existing recv_backend
  • Removes the state entry when fin=true after delivering the frame
  • The ordering (send first at line 220, then remove at line 222) ensures the final frame is delivered before cleanup

@kixelated kixelated merged commit d7b7f2c into main Oct 25, 2025
1 check passed
@kixelated kixelated deleted the insert_entry branch October 25, 2025 19:17
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