-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes to fix #30 #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/mux/stream.rs:198
- The poll_flush implementation immediately returns Ok(()) without flushing any buffered frames. Please confirm that bypassing flush here is intended and does not lead to potential inconsistencies in stream data delivery.
#[tracing::instrument(skip(_cx), level = "trace")]
fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<io::Result<()>> {
src/mux/inner.rs:210
- In the wind_down function, remaining frames are drained using try_recv and sent via ws.feed, but errors are only logged if sending fails. Consider adding more detailed logging or error handling to capture failures when draining remaining frames for improved diagnostics.
if should_drain_frame_rx {
while let Ok(frame) = frame_rx.try_recv() {
src/client/mod.rs:267
- [nitpick] The keepalive parameter has been changed from a u64 to OptionalDuration. Please ensure that all downstream components treat OptionalDuration correctly when converting to a concrete Duration value, to avoid unexpected behavior.
let mut backoff = Backoff::new(Duration::from_millis(200), Duration::from_millis(args.max_retry_interval), 2, args.max_retry_count);
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
+ Coverage 77.63% 79.37% +1.74%
==========================================
Files 31 30 -1
Lines 5370 5160 -210
==========================================
- Hits 4169 4096 -73
+ Misses 1201 1064 -137 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
(cherry picked from commit eb2a6fd) Signed-off-by: Zhang Maiyun <[email protected]>
Signed-off-by: Zhang Maiyun <[email protected]>
5298a55
to
1eef54e
Compare
Confirmed with various |
Signed-off-by: Zhang Maiyun <[email protected]>
Rough testing seems to indicate that 3056b2c has no performance issue over TLS over an internet link
After
|
Signed-off-by: Zhang Maiyun <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/mux/stream.rs:158
- The early return with a BrokenPipe error bypasses any flush or cleanup logic that might be necessary for pending writes. Please add a comment or consider whether the flush should occur before returning the error to ensure graceful shutdown.
if !self.can_write.load(Ordering::Relaxed) {
Signed-off-by: Zhang Maiyun <[email protected]>
Signed-off-by: Zhang Maiyun <[email protected]>
Backported to Penguin v0.5x from the development version.
(cherry picked from commit eb2a6fd)