chore(ups): add message chunking & ups protocol #2874
Closed
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
This was referenced Sep 5, 2025
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
commit: |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+365
to
+391
| async fn next(&mut self) -> Result<DriverOutput> { | ||
| loop { | ||
| tokio::select! { | ||
| biased; | ||
| // Prefer local messages to reduce latency | ||
| res = self.local_rx.recv() => { | ||
| match res { | ||
| std::result::Result::Ok(payload) => { | ||
| return Ok(DriverOutput::Message { subject: self.subject.clone(), payload }); | ||
| } | ||
| std::result::Result::Err(broadcast::error::RecvError::Lagged(_)) => { | ||
| // Skip lagged and continue | ||
| continue; | ||
| } | ||
| std::result::Result::Err(broadcast::error::RecvError::Closed) => { | ||
| // Local channel closed; fall back to driver only | ||
| // Replace with a closed receiver to avoid busy loop | ||
| // We simply continue and rely on driver | ||
| } | ||
| } | ||
| } | ||
| res = self.driver.next() => { | ||
| return res; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
There appears to be a potential issue in the error handling for broadcast::error::RecvError::Closed. The comment indicates that the code should replace the receiver with a closed one to avoid a busy loop, but the implementation doesn't actually do this. When the local channel is closed, the code continues the loop without modifying self.local_rx, which means it will repeatedly hit the same closed channel error.
Consider replacing self.local_rx with a permanently closed receiver when this error is encountered, perhaps by creating a new closed channel:
Err(broadcast::error::RecvError::Closed) => {
// Replace with a permanently closed receiver to avoid busy loop
let (tx, rx) = broadcast::channel::<Vec<u8>>(1);
drop(tx); // Close the channel
self.local_rx = rx;
continue;
}
Suggested change
| async fn next(&mut self) -> Result<DriverOutput> { | |
| loop { | |
| tokio::select! { | |
| biased; | |
| // Prefer local messages to reduce latency | |
| res = self.local_rx.recv() => { | |
| match res { | |
| std::result::Result::Ok(payload) => { | |
| return Ok(DriverOutput::Message { subject: self.subject.clone(), payload }); | |
| } | |
| std::result::Result::Err(broadcast::error::RecvError::Lagged(_)) => { | |
| // Skip lagged and continue | |
| continue; | |
| } | |
| std::result::Result::Err(broadcast::error::RecvError::Closed) => { | |
| // Local channel closed; fall back to driver only | |
| // Replace with a closed receiver to avoid busy loop | |
| // We simply continue and rely on driver | |
| } | |
| } | |
| } | |
| res = self.driver.next() => { | |
| return res; | |
| } | |
| } | |
| } | |
| } | |
| async fn next(&mut self) -> Result<DriverOutput> { | |
| loop { | |
| tokio::select! { | |
| biased; | |
| // Prefer local messages to reduce latency | |
| res = self.local_rx.recv() => { | |
| match res { | |
| std::result::Result::Ok(payload) => { | |
| return Ok(DriverOutput::Message { subject: self.subject.clone(), payload }); | |
| } | |
| std::result::Result::Err(broadcast::error::RecvError::Lagged(_)) => { | |
| // Skip lagged and continue | |
| continue; | |
| } | |
| std::result::Result::Err(broadcast::error::RecvError::Closed) => { | |
| // Local channel closed; fall back to driver only | |
| // Replace with a closed receiver to avoid busy loop | |
| let (tx, rx) = broadcast::channel::<Vec<u8>>(1); | |
| drop(tx); // Close the channel | |
| self.local_rx = rx; | |
| continue; | |
| } | |
| } | |
| } | |
| res = self.driver.next() => { | |
| return res; | |
| } | |
| } | |
| } | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
NathanFlurry
force-pushed
the
09-05-chore_ups_add_message_chunking_ups_protocol
branch
from
9d59211 to
bf98d22
Compare
NathanFlurry
force-pushed
the
09-04-chore_runner_connect_tunnel_before_connecting_pb_ws
branch
from
cf0534f to
90ce950
Compare
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
graphite-app
bot
force-pushed
the
09-04-chore_runner_connect_tunnel_before_connecting_pb_ws
branch
2 times, most recently
from
c140b96 to
f452a88
Compare
MasterPtato
approved these changes
Sep 8, 2025
NathanFlurry
force-pushed
the
09-05-chore_ups_add_message_chunking_ups_protocol
branch
from
bf98d22 to
c9cff29
Compare
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
This was referenced Sep 9, 2025
NathanFlurry
force-pushed
the
09-05-chore_ups_add_message_chunking_ups_protocol
branch
from
c9cff29 to
a294c2e
Compare
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Merge activity
|
graphite-app bot
pushed a commit
that referenced
this pull request
Sep 9, 2025
Fixes RVT-5129 Fixes RVT-5118
graphite-app
bot
deleted the
09-05-chore_ups_add_message_chunking_ups_protocol
branch
This was referenced Sep 12, 2025
This was referenced Sep 12, 2025
Closed
Closed
This was referenced Sep 13, 2025
This was referenced Sep 15, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes RVT-5129
Fixes RVT-5118