-
Notifications
You must be signed in to change notification settings - Fork 39
Avoid some spurious semver changes and bump the rest #121
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
Conversation
No need to confuse people.
WalkthroughThis pull request updates workspace dependencies and package versions across multiple web-transport crates. The primary changes involve renaming stream termination methods in the public API: Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web-transport-quiche/src/send.rs (1)
78-82: Update Drop comment/log to reflectfinish/reset, notcloseThe Drop path now implicitly handles streams where
finishorresethas already been called, but the comment and log still reference a non-existentclosemethod:
- Comment: “without calling
closeorreset”- Log: “stream dropped without
closeorreset”To avoid confusion for users of this API, these should mention
finishinstead ofclose.impl Drop for SendStream { fn drop(&mut self) { - // Reset the stream if we dropped without calling `close` or `reset` + // Reset the stream if we dropped without calling `finish` or `reset` if !self.inner.is_finished().unwrap_or(true) { - tracing::warn!("stream dropped without `close` or `reset`"); + tracing::warn!("stream dropped without `finish` or `reset`"); self.inner.reset(DROP_CODE) } } }web-transport-quiche/src/recv.rs (1)
55-57:stopmapping and trait delegation are consistent with new API
RecvStream::stopnow correctly maps WebTransport’su32error code into the HTTP/3 space and calls the inner quiche-layer stop, and the trait implementation delegates to this method as expected. This lines up with the trait rename fromclosetostop.For clarity, you could make the trait delegation explicitly call the inherent method:
impl web_transport_trait::RecvStream for RecvStream { @@ - fn stop(&mut self, code: u32) { - self.stop(code); - } + fn stop(&mut self, code: u32) { + RecvStream::stop(self, code); + } }Also applies to: 97-99
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
Cargo.toml(1 hunks)web-transport-proto/Cargo.toml(1 hunks)web-transport-quiche/Cargo.toml(1 hunks)web-transport-quiche/src/ez/recv.rs(3 hunks)web-transport-quiche/src/ez/send.rs(5 hunks)web-transport-quiche/src/recv.rs(3 hunks)web-transport-quiche/src/send.rs(3 hunks)web-transport-quinn/src/error.rs(4 hunks)web-transport-quinn/src/recv.rs(1 hunks)web-transport-quinn/src/send.rs(1 hunks)web-transport-quinn/src/session.rs(2 hunks)web-transport-trait/Cargo.toml(1 hunks)web-transport-trait/src/lib.rs(2 hunks)web-transport-ws/src/session.rs(4 hunks)web-transport/Cargo.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
web-transport-quiche/src/ez/recv.rs (5)
web-transport-quiche/src/recv.rs (2)
stop(55-57)stop(97-99)web-transport-quinn/src/recv.rs (2)
stop(24-28)stop(105-107)web-transport-trait/src/lib.rs (1)
stop(228-228)web-transport-ws/src/session.rs (1)
stop(717-723)web-transport/src/quinn.rs (1)
stop(279-281)
web-transport-quiche/src/recv.rs (6)
web-transport-quiche/src/ez/recv.rs (1)
stop(304-311)web-transport-quinn/src/recv.rs (2)
stop(24-28)stop(105-107)web-transport-trait/src/lib.rs (1)
stop(228-228)web-transport-ws/src/session.rs (1)
stop(717-723)web-transport/src/quinn.rs (1)
stop(279-281)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-trait/src/lib.rs (10)
web-transport-quiche/src/ez/send.rs (2)
finish(287-307)reset(317-324)web-transport-quinn/src/send.rs (4)
finish(77-79)finish(131-133)reset(27-31)reset(127-129)web-transport-ws/src/session.rs (3)
finish(585-608)reset(573-583)stop(717-723)web-transport-wasm/src/send.rs (2)
finish(45-48)reset(38-40)web-transport/src/quinn.rs (3)
finish(216-221)reset(209-211)stop(279-281)web-transport/src/wasm.rs (3)
finish(142-145)reset(135-137)stop(167-169)web-transport-quiche/src/ez/recv.rs (1)
stop(304-311)web-transport-quiche/src/recv.rs (2)
stop(55-57)stop(97-99)web-transport-quinn/src/recv.rs (2)
stop(24-28)stop(105-107)web-transport-wasm/src/recv.rs (1)
stop(69-71)
web-transport-quiche/src/ez/send.rs (5)
web-transport-quiche/src/send.rs (2)
reset(65-68)reset(121-123)web-transport-quinn/src/send.rs (2)
reset(27-31)reset(127-129)web-transport-trait/src/lib.rs (1)
reset(155-155)web-transport-ws/src/session.rs (1)
reset(573-583)web-transport/src/quinn.rs (1)
reset(209-211)
web-transport-quinn/src/recv.rs (5)
web-transport-quiche/src/ez/recv.rs (1)
stop(304-311)web-transport-quiche/src/recv.rs (2)
stop(55-57)stop(97-99)web-transport-trait/src/lib.rs (1)
stop(228-228)web-transport-ws/src/session.rs (1)
stop(717-723)web-transport/src/quinn.rs (1)
stop(279-281)
web-transport-ws/src/session.rs (11)
web-transport-quiche/src/ez/send.rs (1)
reset(317-324)web-transport-quiche/src/send.rs (2)
reset(65-68)reset(121-123)web-transport-quinn/src/send.rs (2)
reset(27-31)reset(127-129)web-transport-trait/src/lib.rs (2)
reset(155-155)stop(228-228)web-transport-wasm/src/send.rs (1)
reset(38-40)web-transport/src/quinn.rs (2)
reset(209-211)stop(279-281)web-transport/src/wasm.rs (2)
reset(135-137)stop(167-169)web-transport-ws/src/session.ts (1)
code(393-406)web-transport-quiche/src/ez/recv.rs (1)
stop(304-311)web-transport-quiche/src/recv.rs (2)
stop(55-57)stop(97-99)web-transport-quinn/src/recv.rs (2)
stop(24-28)stop(105-107)
web-transport-quiche/src/send.rs (6)
web-transport-quiche/src/ez/send.rs (1)
reset(317-324)web-transport-quinn/src/send.rs (2)
reset(27-31)reset(127-129)web-transport-trait/src/lib.rs (1)
reset(155-155)web-transport-ws/src/session.rs (1)
reset(573-583)web-transport/src/quinn.rs (1)
reset(209-211)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-quinn/src/send.rs (7)
web-transport-quiche/src/ez/send.rs (1)
reset(317-324)web-transport-quiche/src/send.rs (2)
reset(65-68)reset(121-123)web-transport-trait/src/lib.rs (1)
reset(155-155)web-transport-ws/src/session.rs (1)
reset(573-583)web-transport-wasm/src/send.rs (1)
reset(38-40)web-transport/src/quinn.rs (1)
reset(209-211)web-transport/src/wasm.rs (1)
reset(135-137)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (23)
web-transport-trait/Cargo.toml (1)
8-8: LGTM! Version bump aligns with breaking API changes.The 0.2.0 → 0.3.0 version bump is appropriate for the breaking changes to the
SendStreamandRecvStreamtrait methods (close→reset/stop).web-transport-proto/Cargo.toml (1)
8-8: LGTM! Version bump aligns with workspace updates.The 0.2.8 → 0.3.0 version bump aligns with the workspace-wide updates, maintaining consistent versioning across related crates.
web-transport/Cargo.toml (1)
8-8: LGTM! Version bump reflects breaking changes in dependencies.The 0.9.7 → 0.10.0 version bump is appropriate as this crate re-exports APIs from web-transport-quinn, which underwent breaking changes.
Cargo.toml (1)
14-15: LGTM! Workspace dependency versions align with crate updates.The workspace dependency updates for web-transport-proto and web-transport-trait to version 0.3 are consistent with the breaking API changes across the workspace.
web-transport-quinn/src/send.rs (1)
127-129: LGTM! Trait method implementation correctly renamed.The trait method implementation is correctly renamed from
closetoreset, properly delegating to the existingSelf::reset(self, code)method. The.ok()appropriately converts theResultto unit return type as required by the trait.web-transport-quinn/src/recv.rs (1)
105-107: LGTM! Trait method implementation correctly renamed.The trait method implementation is correctly renamed from
closetostop, properly delegating to the existingSelf::stop(self, code)method. The.ok()appropriately converts theResultto unit return type as required by the trait.web-transport-quiche/src/ez/recv.rs (3)
304-311: LGTM! Method renamed and implementation preserved.The public method is correctly renamed from
closetostopwhile preserving the existing behavior. The implementation properly sets the stop state and wakes the driver.
316-318: LGTM! Documentation updated to reflect method rename.The documentation correctly references
RecvStream::stopinstead of the oldclosemethod name.
338-340: LGTM! Documentation updated consistently.The documentation in the
closed()method correctly referencesRecvStream::stop, maintaining consistency with the renamed API.web-transport-quiche/Cargo.toml (1)
32-32: tokio-quiche 0.12 is available and compatible for release.tokio-quiche versions 0.12.0, 0.11.0, and 0.10.0 are all published on crates.io with no gaps in the version sequence. The original concern that "version 0.11 was skipped" is incorrect—0.11.0 exists and was released on Nov 6, 2025, followed by 0.12.0 on Nov 13, 2025. Verify that your code's usage patterns are compatible with 0.12.0's API if you have any uncertainty.
web-transport-quinn/src/session.rs (1)
427-430: Error mapping now consistent with newWebTransportErrorvariantsMapping
VarInt::readfailures toWebTransportError::UnknownSessionmatches the removal ofUnknownStreamand keeps header-parse failures surfaced at the session level. Behavior looks coherent with the updated error enum.Also applies to: 489-492
web-transport-quiche/src/ez/send.rs (2)
8-8: Import change is harmless and idiomaticSwitching to
use tokio_quiche::quiche::{self};is equivalent to the previous form and keeps the path concise. No issues here.
282-287: Reset/stop semantics and docs align with implementationThe updated docs around
finish,reset,is_closed, andclosednow match the actual behavior: finishing setsfin, resets go throughstate.resetand result inRESET_STREAM, and closed covers reset/stop/FIN cases. Usingu64forresethere (HTTP/3 code) is consistent with the outer wrapper converting from WebTransport’su32space.If you want extra assurance, re-run tests that exercise
SendStream::finish,SendStream::reset, and drop paths for the quiche backend.Also applies to: 314-324, 329-333, 351-353
web-transport-quiche/src/send.rs (1)
65-67: Reset wiring from WebTransport code to HTTP/3 code looks correct
reset(&mut self, code: u32)converting throughweb_transport_proto::error_to_http3and delegating toinner.resetmatches how other backends handle error codes and aligns with the trait signature.Consider running any existing tests that assert quiche backend error codes on reset to confirm nothing changed unexpectedly.
Also applies to: 121-123
web-transport-quiche/src/recv.rs (1)
68-70: Drop warning text matches newstopsemanticsThe updated warning (“stream dropped without
stopor reading all contents”) accurately reflects the new RecvStream API and the fact that Drop now sends a STOP_SENDING code if the stream wasn’t fully drained or explicitly stopped. This behavior is consistent with the trait docs.web-transport-trait/src/lib.rs (1)
144-149: Trait rename toreset/stopis correctly implemented and coherentThe trait-level changes have been successfully applied.
SendStreamnow exposesfinish(),reset(), andclosed()methods, whileRecvStreamhasstop()andclosed(). All implementations (quinn, quiche, wasm, ws) correctly implement these new trait methods. Theclose()methods appearing in grep results are session-level convenience wrappers, not trait methods, so they pose no compatibility issues.web-transport-ws/src/session.rs (4)
521-527: LGTM! Drop implementation correctly updated.The Drop implementation properly calls the renamed
resetmethod, maintaining the same behavior of resetting the stream with code 0 when dropped without explicit termination.
573-583: LGTM! Method rename improves semantic clarity.The rename from
closetoresetbetter conveys that this is an ungraceful stream termination. The implementation logic is correctly preserved and aligns with the updated trait definition.
653-659: LGTM! Drop implementation correctly updated.The Drop implementation properly calls the renamed
stopmethod, maintaining the same behavior of stopping the stream with code 0 when dropped without explicit termination.
717-723: LGTM! Method rename improves semantic clarity.The rename from
closetostopbetter conveys that this is a receive-side stream termination signal. The implementation logic is correctly preserved and aligns with the updated trait definition.web-transport-quinn/src/error.rs (3)
46-46: LGTM! Enum type reference correctly updated.The field correctly references the renamed
WebTransportErrortype with the#[from]attribute preserved for automatic error conversion.
52-68: LGTM! Error conversion correctly updated.The conversion from Quinn's connection error properly uses the renamed
Closedvariant. The catch-all pattern on Line 65 appropriately handles all other connection error cases, including any previously explicitly-handled variants.
72-74: LGTM! Variant definition and usage are consistent.The
Closedvariant is well-defined with a clear error message on Line 73. The trait implementation on Lines 258-266 correctly pattern matches and extracts the session error information, maintaining consistency throughout the error handling flow.Also applies to: 258-266
Summary by CodeRabbit
Breaking Changes
close()toreset()close()tostop()Dependencies
Version Updates