Initial web-transport-quiche support #118
Conversation
WalkthroughAdds a new workspace member Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-transport-ws/src/session.rs (1)
585-603: Propagate channel closure fromfinish().When
outbound.try_send(...)returnsTrySendError::Closed, the spawned task will also fail, yet we still setself.fin = trueand reportOk(()). That causes callers to believe FIN was sent even though the session is already closed. Detect a closed channel immediately, mark the stream closed, and returnError::Closed(or the stored error) instead of spawning a task.- if let Err(e) = self.outbound.try_send(frame.into()) { - // This is a sync function so we need to spawn a task if we're blocked on sending the frame. - // Thanks, I hate it. - let outbound = self.outbound.clone(); - tokio::spawn(async move { - outbound.send(e.into_inner()).await.ok(); - }); - } + if let Err(e) = self.outbound.try_send(frame.into()) { + let frame = e.into_inner(); + if self.outbound.is_closed() { + self.closed = Some(Error::Closed); + return Err(Error::Closed); + } + let outbound = self.outbound.clone(); + tokio::spawn(async move { + if outbound.send(frame).await.is_err() { + tracing::debug!("dropping FIN: outbound channel closed"); + } + }); + }
🧹 Nitpick comments (6)
web-transport-quiche/README.md (1)
1-15: Fix the README to reference the quiche crate, not quinn.Both badges still point to
web-transport-quinn, and the intro paragraph says we delegate to “the underlying QUIC implementation (Quinn)”. For this crate we should link to and nameweb-transport-quiche. Please update the badge URLs/text and the sentence describing the QUIC implementation accordingly.web-transport-quiche/examples/echo-server.rs (1)
77-92: Consider adding size limits for production use.While appropriate for an example, the unbounded
loopandrecv.read_all()combination could cause issues in production:
- No explicit loop termination means errors are the only exit path
read_all()without size limits could enable DoS attacks with large messagesFor production code, consider:
- Adding a maximum message size parameter to
read_all()- Implementing timeouts for stream operations
- Adding a connection idle timeout
web-transport-proto/src/varint.rs (1)
206-223: Clarify the "Super jaink" comment.Line 210's comment "Super jaink but keeps everything on the stack" is unclear. Consider rewording for clarity, e.g., "Hacky but keeps everything on the stack" or explaining what "jaink" means in this context.
web-transport-proto/src/settings.rs (1)
171-180: TODO: Heap allocation noted.The TODO on line 172 acknowledges the heap allocation. If performance is critical, consider using a fixed-size stack buffer with a reasonable maximum settings size, similar to the VarInt write approach.
web-transport-quiche/src/connection.rs (1)
171-173: Consider preserving error details when mapping to SessionError::Header.Lines 171-173 and 182-184 map write errors to
SessionError::Headerusing.map_err(|_| SessionError::Header), discarding the underlying error. This could make debugging difficult if header writes fail for specific reasons (e.g., connection closed, buffer full).Consider logging the error before mapping:
- send.write_all(&self.header_uni) - .await - .map_err(|_| SessionError::Header)?; + send.write_all(&self.header_uni) + .await + .map_err(|e| { + tracing::warn!("failed to write stream header: {e}"); + SessionError::Header + })?;Also applies to: 182-184
web-transport-quiche/src/ez/connection.rs (1)
186-202: Consider implementing backpressure for stream opens.The TODOs on lines 192 and 208 indicate that
open_biandopen_unicurrently don't wait for the driver to acknowledge stream creation. If the application opens streams faster than the driver can process them, this could lead to:
- Unbounded buffering in the open channels
- Memory growth
- Potential deadlocks if channels fill up
Since the channels are
flume::bounded(1)(from server.rs line 170-171), opening multiple streams concurrently could block unexpectedly.Also applies to: 204-216
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web-demo/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (49)
Cargo.toml(1 hunks)web-demo/client.js(1 hunks)web-transport-proto/Cargo.toml(1 hunks)web-transport-proto/src/capsule.rs(3 hunks)web-transport-proto/src/connect.rs(5 hunks)web-transport-proto/src/error.rs(1 hunks)web-transport-proto/src/settings.rs(4 hunks)web-transport-proto/src/varint.rs(3 hunks)web-transport-quiche/Cargo.toml(1 hunks)web-transport-quiche/README.md(1 hunks)web-transport-quiche/examples/README.md(1 hunks)web-transport-quiche/examples/echo-client.rs(1 hunks)web-transport-quiche/examples/echo-server.rs(1 hunks)web-transport-quiche/src/client.rs(1 hunks)web-transport-quiche/src/connection.rs(1 hunks)web-transport-quiche/src/error.rs(1 hunks)web-transport-quiche/src/ez/client.rs(1 hunks)web-transport-quiche/src/ez/connection.rs(1 hunks)web-transport-quiche/src/ez/driver.rs(1 hunks)web-transport-quiche/src/ez/lock.rs(1 hunks)web-transport-quiche/src/ez/mod.rs(1 hunks)web-transport-quiche/src/ez/recv.rs(1 hunks)web-transport-quiche/src/ez/send.rs(1 hunks)web-transport-quiche/src/ez/server.rs(1 hunks)web-transport-quiche/src/ez/stream.rs(1 hunks)web-transport-quiche/src/h3/connect.rs(1 hunks)web-transport-quiche/src/h3/mod.rs(1 hunks)web-transport-quiche/src/h3/request.rs(1 hunks)web-transport-quiche/src/h3/settings.rs(1 hunks)web-transport-quiche/src/lib.rs(1 hunks)web-transport-quiche/src/recv.rs(1 hunks)web-transport-quiche/src/send.rs(1 hunks)web-transport-quiche/src/server.rs(1 hunks)web-transport-quinn/examples/README.md(1 hunks)web-transport-quinn/examples/echo-client.rs(1 hunks)web-transport-quinn/src/connect.rs(2 hunks)web-transport-quinn/src/error.rs(2 hunks)web-transport-quinn/src/recv.rs(1 hunks)web-transport-quinn/src/send.rs(4 hunks)web-transport-quinn/src/server.rs(1 hunks)web-transport-quinn/src/session.rs(6 hunks)web-transport-quinn/src/settings.rs(2 hunks)web-transport-quinn/web/client.js(0 hunks)web-transport-trait/src/lib.rs(6 hunks)web-transport-ws/examples/client.rs(2 hunks)web-transport-ws/examples/server.rs(2 hunks)web-transport-ws/src/error.rs(1 hunks)web-transport-ws/src/session.rs(9 hunks)web-transport/src/quinn.rs(2 hunks)
💤 Files with no reviewable changes (1)
- web-transport-quinn/web/client.js
🧰 Additional context used
🧬 Code graph analysis (35)
web-transport-ws/src/error.rs (3)
web-transport-quinn/src/error.rs (5)
session_error(266-273)session_error(277-283)session_error(294-300)stream_error(285-290)stream_error(302-307)web-transport-trait/src/lib.rs (2)
session_error(15-15)stream_error(18-20)web-transport-quiche/src/error.rs (3)
session_error(78-84)session_error(94-100)stream_error(86-91)
web-transport-quinn/src/recv.rs (2)
web-transport-trait/src/lib.rs (3)
close(67-67)close(155-155)close(228-228)web-transport-quinn/src/send.rs (1)
close(127-129)
web-transport-quiche/examples/echo-server.rs (2)
web-transport-quiche/src/connection.rs (1)
url(286-288)web-transport-quiche/src/h3/request.rs (1)
url(30-32)
web-transport-quinn/src/error.rs (5)
web-transport-ws/src/error.rs (4)
from(40-42)from(46-48)session_error(52-61)stream_error(63-69)web-transport-quiche/src/error.rs (5)
from(45-57)from(61-74)session_error(78-84)session_error(94-100)stream_error(86-91)web-transport/src/quinn.rs (4)
from(73-75)from(168-170)from(320-325)from(328-333)web-transport-trait/src/lib.rs (2)
session_error(15-15)stream_error(18-20)web-transport-proto/src/error.rs (1)
error_from_http3(5-14)
web-transport-quiche/src/h3/request.rs (4)
web-transport-quiche/src/connection.rs (6)
url(286-288)connect(127-139)new(63-100)new(359-381)close(252-260)close(324-326)web-transport-quiche/src/h3/connect.rs (2)
url(99-101)accept(38-52)web-transport-quiche/src/h3/settings.rs (2)
accept(45-56)connect(36-43)web-transport-quiche/src/server.rs (3)
accept(113-126)new(38-40)new(105-110)
web-transport-proto/src/settings.rs (4)
web-transport-proto/src/capsule.rs (4)
read(72-86)decode(20-70)encode(88-120)write(122-130)web-transport-proto/src/connect.rs (8)
read(101-115)read(178-192)decode(60-99)decode(156-176)encode(117-137)encode(194-207)write(139-147)write(209-217)web-transport-proto/src/varint.rs (4)
read(167-189)decode(130-164)encode(191-204)write(206-223)web-transport-proto/src/frame.rs (3)
read(30-48)decode(9-12)encode(14-16)
web-transport-quiche/src/client.rs (3)
web-transport-quiche/src/connection.rs (4)
url(286-288)new(63-100)new(359-381)connect(127-139)web-transport-quiche/src/ez/client.rs (7)
default(20-22)with_metrics(26-36)with_socket(38-56)with_bind(58-62)with_settings(68-71)with_cert(75-82)connect(87-181)web-transport-quiche/src/h3/settings.rs (1)
connect(36-43)
web-transport-quiche/src/ez/recv.rs (4)
web-transport-quiche/src/connection.rs (7)
new(63-100)new(359-381)close(252-260)close(324-326)closed(263-265)closed(328-330)drop(25-30)web-transport-quiche/src/ez/driver.rs (3)
new(71-96)waker(26-33)read(110-185)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)web-transport-quiche/src/ez/send.rs (10)
new(50-62)new(219-221)id(64-66)id(223-225)flush(115-193)is_closed(205-207)is_closed(323-325)close(308-315)closed(336-338)drop(351-364)
web-transport-quiche/src/h3/mod.rs (3)
web-transport-quiche/src/connection.rs (1)
connect(127-139)web-transport-quiche/src/client.rs (1)
connect(73-80)web-transport-quiche/src/h3/settings.rs (1)
connect(36-43)
web-transport-proto/src/varint.rs (4)
web-transport-proto/src/settings.rs (4)
read(135-153)decode(18-20)decode(110-133)write(171-180)web-transport-proto/src/capsule.rs (3)
read(72-86)decode(20-70)write(122-130)web-transport-proto/src/connect.rs (6)
read(101-115)read(178-192)decode(60-99)decode(156-176)write(139-147)write(209-217)web-transport-proto/src/frame.rs (2)
read(30-48)decode(9-12)
web-transport-proto/src/capsule.rs (4)
web-transport-proto/src/connect.rs (6)
read(101-115)read(178-192)decode(60-99)decode(156-176)write(139-147)write(209-217)web-transport-proto/src/settings.rs (4)
read(135-153)decode(18-20)decode(110-133)write(171-180)web-transport-proto/src/varint.rs (3)
read(167-189)decode(130-164)write(206-223)web-transport-proto/src/frame.rs (2)
read(30-48)decode(9-12)
web-transport-quiche/src/ez/send.rs (3)
web-transport-quiche/src/ez/recv.rs (13)
std(151-151)std(263-263)new(54-67)new(223-225)id(69-71)id(227-229)poll_closed(109-120)flush(122-209)is_closed(211-213)is_closed(298-300)close(283-290)closed(310-312)drop(316-329)web-transport-quiche/src/ez/driver.rs (3)
new(71-96)waker(26-33)write(187-216)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)
web-transport-quiche/src/h3/connect.rs (3)
web-transport-quiche/src/connection.rs (3)
url(286-288)new(63-100)new(359-381)web-transport-quiche/src/h3/request.rs (2)
url(30-32)accept(14-27)web-transport-proto/src/connect.rs (2)
read(101-115)read(178-192)
web-transport-quiche/src/error.rs (2)
web-transport-proto/src/error.rs (1)
error_from_http3(5-14)web-transport-trait/src/lib.rs (2)
session_error(15-15)stream_error(18-20)
web-transport-quiche/src/ez/client.rs (3)
web-transport-quiche/src/client.rs (7)
default(30-32)with_metrics(37-39)with_socket(44-46)with_bind(51-55)with_settings(61-63)with_cert(66-68)connect(73-80)web-transport-quiche/src/ez/connection.rs (6)
new(90-92)new(141-167)accept_bi(170-175)accept_uni(178-183)open_bi(186-202)open_uni(205-216)web-transport-quiche/src/ez/driver.rs (3)
new(71-96)open_bi(372-408)open_uni(410-432)
web-transport-proto/src/connect.rs (3)
web-transport-proto/src/capsule.rs (3)
read(72-86)decode(20-70)write(122-130)web-transport-proto/src/settings.rs (4)
read(135-153)decode(18-20)decode(110-133)write(171-180)web-transport-proto/src/varint.rs (3)
read(167-189)decode(130-164)write(206-223)
web-transport-quinn/src/connect.rs (1)
web-transport-proto/src/connect.rs (2)
read(101-115)read(178-192)
web-transport-quinn/src/settings.rs (1)
web-transport-proto/src/settings.rs (1)
read(135-153)
web-transport-quiche/src/recv.rs (2)
web-transport-quiche/src/ez/recv.rs (12)
std(151-151)std(263-263)new(54-67)new(223-225)read(231-236)read_chunk(238-240)read_buf(260-273)read_all(275-280)close(283-290)closed(310-312)drop(316-329)poll_read(333-344)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-quiche/src/server.rs (5)
web-transport-quiche/src/ez/server.rs (14)
new(41-47)new(141-157)default(35-37)with_listener(57-59)with_listener(82-85)with_socket(61-66)with_socket(87-104)with_bind(68-73)with_bind(106-110)with_settings(75-78)with_settings(112-115)with_cert(119-129)accept(213-215)next(49-55)web-transport-quinn/src/server.rs (3)
default(26-28)accept(106-123)accept(135-148)web-transport-quiche/src/h3/connect.rs (1)
accept(38-52)web-transport-quiche/src/h3/request.rs (1)
accept(14-27)web-transport-quiche/src/h3/settings.rs (1)
accept(45-56)
web-transport-proto/src/error.rs (1)
web-transport-wasm/src/error.rs (1)
code(21-26)
web-transport-ws/examples/server.rs (4)
web-transport-quiche/src/ez/send.rs (2)
finish(280-300)closed(336-338)web-transport-trait/src/lib.rs (4)
finish(149-149)closed(70-70)closed(167-167)closed(236-236)web-transport-quinn/src/send.rs (3)
finish(77-79)finish(131-133)closed(151-157)web-transport-ws/src/session.rs (4)
finish(585-608)closed(466-473)closed(610-621)closed(725-757)
web-transport-quiche/src/ez/mod.rs (1)
web-transport-quiche/src/ez/lock.rs (1)
lock(28-31)
web-transport-ws/src/session.rs (8)
web-transport-quiche/src/connection.rs (4)
closed(263-265)closed(328-330)close(252-260)close(324-326)web-transport-quiche/src/ez/recv.rs (2)
closed(310-312)close(283-290)web-transport-quiche/src/ez/send.rs (4)
closed(336-338)close(308-315)set_priority(340-347)finish(280-300)web-transport-trait/src/lib.rs (8)
closed(70-70)closed(167-167)closed(236-236)close(67-67)close(155-155)close(228-228)set_priority(140-140)finish(149-149)web-transport-quinn/src/recv.rs (2)
closed(119-122)close(105-107)web-transport-quinn/src/send.rs (6)
closed(151-157)close(127-129)set_priority(81-83)set_priority(123-125)finish(77-79)finish(131-133)web-transport-quinn/src/session.rs (4)
closed(261-263)closed(524-526)close(248-258)close(520-522)web-transport-wasm/src/session.rs (2)
closed(102-104)close(94-99)
web-transport-trait/src/lib.rs (11)
web-transport-quinn/src/error.rs (5)
session_error(266-273)session_error(277-283)session_error(294-300)stream_error(285-290)stream_error(302-307)web-transport-ws/src/error.rs (2)
session_error(52-61)stream_error(63-69)web-transport-quiche/src/connection.rs (4)
closed(263-265)closed(328-330)close(252-260)close(324-326)web-transport-quiche/src/ez/connection.rs (2)
closed(236-238)close(222-230)web-transport-quiche/src/recv.rs (4)
closed(45-47)closed(86-88)close(41-43)close(82-84)web-transport-quiche/src/send.rs (7)
closed(55-57)set_priority(46-48)set_priority(101-103)finish(42-44)finish(109-111)close(50-53)close(105-107)web-transport-quinn/src/session.rs (4)
closed(261-263)closed(524-526)close(248-258)close(520-522)web-transport-wasm/src/send.rs (3)
closed(59-73)set_priority(53-56)finish(45-48)web-transport-wasm/src/session.rs (2)
closed(102-104)close(94-99)web-transport-wasm/src/recv.rs (1)
closed(74-88)web-transport-quinn/src/send.rs (4)
set_priority(81-83)set_priority(123-125)finish(77-79)finish(131-133)
web-transport-quiche/src/ez/stream.rs (4)
web-transport-quiche/src/ez/connection.rs (2)
new(90-92)new(141-167)web-transport-quiche/src/ez/driver.rs (1)
new(71-96)web-transport-quiche/src/recv.rs (1)
new(21-23)web-transport-quiche/src/send.rs (1)
new(22-24)
web-transport-quiche/src/ez/lock.rs (3)
web-transport-quiche/src/ez/recv.rs (2)
new(54-67)new(223-225)web-transport-quiche/src/connection.rs (2)
new(63-100)new(359-381)web-transport-quiche/src/ez/send.rs (2)
new(50-62)new(219-221)
web-transport-quinn/src/send.rs (3)
web-transport-quiche/src/send.rs (4)
set_priority(46-48)set_priority(101-103)finish(42-44)finish(109-111)web-transport-trait/src/lib.rs (5)
set_priority(140-140)close(67-67)close(155-155)close(228-228)finish(149-149)web-transport/src/quinn.rs (4)
set_priority(204-206)close(151-153)reset(209-211)finish(216-221)
web-transport-quiche/src/ez/driver.rs (5)
web-transport-quiche/src/ez/recv.rs (7)
std(151-151)std(263-263)new(54-67)new(223-225)closed(310-312)id(69-71)id(227-229)web-transport-quiche/src/ez/connection.rs (6)
new(90-92)new(141-167)closed(236-238)wait(75-77)poll(64-73)abort(53-61)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)web-transport-quiche/src/ez/send.rs (5)
new(50-62)new(219-221)closed(336-338)id(64-66)id(223-225)web-transport-quiche/src/ez/stream.rs (3)
from(58-60)from(64-66)from(70-72)
web-transport-quiche/src/ez/server.rs (3)
web-transport-quiche/src/server.rs (13)
ServerBuilder(46-46)ServerBuilder(53-53)ServerBuilder(62-62)default(32-34)new(38-40)new(105-110)with_listener(42-47)with_listener(73-75)with_socket(49-56)with_socket(77-79)with_bind(58-65)with_bind(81-83)accept(113-126)web-transport-quiche/src/ez/connection.rs (6)
new(90-92)new(141-167)accept_bi(170-175)accept_uni(178-183)open_bi(186-202)open_uni(205-216)web-transport-quiche/src/ez/driver.rs (3)
new(71-96)open_bi(372-408)open_uni(410-432)
web-transport-quiche/src/ez/connection.rs (5)
web-transport-quiche/src/ez/recv.rs (9)
new(54-67)new(223-225)is_closed(211-213)is_closed(298-300)closed(310-312)drop(316-329)id(69-71)id(227-229)close(283-290)web-transport-quiche/src/ez/driver.rs (7)
abort(434-439)new(71-96)poll(222-370)waker(26-33)wait(218-220)open_bi(372-408)open_uni(410-432)web-transport-quiche/src/connection.rs (15)
new(63-100)new(359-381)closed(263-265)closed(328-330)drop(25-30)accept_bi(155-165)accept_bi(300-302)accept_uni(142-152)accept_uni(296-298)open_bi(179-187)open_bi(304-306)open_uni(168-176)open_uni(308-310)close(252-260)close(324-326)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)clone(14-18)web-transport-quiche/src/ez/send.rs (9)
new(50-62)new(219-221)is_closed(205-207)is_closed(323-325)closed(336-338)drop(351-364)id(64-66)id(223-225)close(308-315)
web-transport-quiche/src/send.rs (2)
web-transport-proto/src/error.rs (1)
error_to_http3(16-18)web-transport-quiche/src/ez/send.rs (13)
new(50-62)new(219-221)write(227-230)write_buf(263-265)write_all(252-258)write_buf_all(268-273)finish(280-300)close(308-315)closed(336-338)drop(351-364)poll_write(368-378)poll_flush(380-383)poll_shutdown(385-388)
web-transport-quinn/src/session.rs (3)
web-transport-proto/src/capsule.rs (2)
read(72-86)decode(20-70)web-transport-trait/src/lib.rs (4)
read(180-183)closed(70-70)closed(167-167)closed(236-236)web-transport-proto/src/varint.rs (2)
read(167-189)decode(130-164)
web-transport-quiche/src/h3/settings.rs (3)
web-transport-quiche/src/connection.rs (1)
connect(127-139)web-transport-quiche/src/h3/connect.rs (2)
accept(38-52)open(68-91)web-transport-quiche/src/h3/request.rs (1)
accept(14-27)
web-transport-quiche/src/connection.rs (5)
web-transport-quiche/src/h3/connect.rs (4)
url(99-101)session_id(94-96)accept(38-52)open(68-91)web-transport-quinn/src/session.rs (17)
url(295-297)connect(116-128)run_closed(96-112)accept_uni(131-141)accept_uni(504-506)accept_bi(144-154)accept_bi(508-510)open_uni(157-169)open_uni(516-518)open_bi(172-184)open_bi(512-514)close(248-258)close(520-522)closed(261-263)closed(524-526)poll_accept_uni(375-414)poll_accept_bi(441-471)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)web-transport-quiche/src/ez/connection.rs (9)
drop(96-101)new(90-92)new(141-167)accept_uni(178-183)accept_bi(170-175)open_uni(205-216)open_bi(186-202)close(222-230)closed(236-238)web-transport-quiche/src/h3/settings.rs (3)
connect(36-43)accept(45-56)open(58-68)
🪛 LanguageTool
web-transport-quiche/README.md
[style] ~7-~7: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1544 characters long)
Context: ...des a QUIC-like API but with web support! ## WebTransport [WebTransport](https:/...
(EN_EXCESSIVE_EXCLAMATION)
⏰ 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 (24)
web-transport-ws/examples/server.rs (2)
50-50: LGTM: Synchronousfinish()calls are correct.The removal of
.awaiton bothfinish()calls correctly reflects the trait signature change fromasync fn finish()tofn finish(). The synchronous version marks the stream as finished locally and returns immediately, while any required I/O is handled asynchronously in the background.Also applies to: 62-62
67-68: LGTM: Session closure handling is correct.The error handling correctly reflects the API change where
Session::closed()now returnsSelf::Errordirectly rather thanResult<(), Self::Error>. The patternerr = session.closed()appropriately captures the error value and propagates it through the function's error handling.web-transport-ws/examples/client.rs (1)
17-17: LGTM! Synchronous finish() simplifies the API.The transition from asynchronous to synchronous
finish()calls streamlines the stream lifecycle management. Both unidirectional and bidirectional stream examples now use the simpler synchronous API while maintaining proper error propagation.Also applies to: 36-36
web-transport-quinn/examples/echo-client.rs (1)
81-82: Good addition of explicit session closure.The explicit
close()andclosed().awaitsequence ensures graceful session teardown before the client exits, which is a best practice for demonstrating proper lifecycle management in examples.web-transport-quinn/src/server.rs (1)
156-165: LGTM! Consistent error handling with ServerError.The unified
ServerErrortype for bothok()andclose()methods provides consistent error propagation across the server handshake flow. The async signatures are appropriate for I/O operations.web-transport-quiche/Cargo.toml (1)
1-42: LGTM! Well-structured manifest for the new crate.The dependency selection is appropriate for a QUIC-based WebTransport implementation. The selective Tokio features in production dependencies help minimize binary size while enabling full features for examples and tests.
web-transport/src/quinn.rs (2)
82-82: LGTM! Simplified error handling in accept path.The direct
awaitand?propagation is cleaner than the previous intermediate error mapping, enabled by the newServerErrorvariant below.
306-307: Good addition of ServerError variant.This enables seamless
Fromconversion fromquinn::ServerError, supporting the cleaner error propagation in theaccept()method above.web-transport-quinn/src/settings.rs (2)
48-56: LGTM! Cleaner SETTINGS handling with async helpers.The switch to
web_transport_proto::Settings::read()removes manual buffering logic and makes the code more maintainable. The WebTransport support validation and debug logging are helpful additions.
66-66: Good use of async write helper.Direct
settings.write()eliminates the need for intermediate buffer allocation, streamlining the send path.web-transport-quinn/examples/README.md (1)
9-12: LGTM! Documentation updated for new directory structure.The path updates consistently reflect the reorganized dev/demo layout, ensuring developers can follow the instructions successfully.
web-transport-quiche/examples/echo-server.rs (2)
35-45: Good error context for path conversion.The
.context()calls provide helpful error messages when TLS certificate paths are invalid.
54-63: LGTM! Standard accept-and-spawn pattern.The connection handling loop properly spawns per-connection tasks and logs useful information for debugging.
web-transport-quiche/examples/echo-client.rs (1)
1-60: LGTM! Clear example demonstrating the client API.The example effectively demonstrates the client-side WebTransport flow: connection setup with TLS configuration, bidirectional stream creation, message exchange, and graceful session closure. The code is clean and well-structured for an example.
web-transport-proto/src/capsule.rs (1)
122-130: LGTM! Standard async write pattern.The write method follows the established pattern of encoding to a buffer and writing atomically.
web-transport-proto/src/varint.rs (1)
166-189: Verify the unwrap() is safe or replace with proper error handling.Line 186 uses
unwrap()assuming thatVarInt::decode()will always succeed after reading the exact number of bytes based on the size prefix. While this should be safe given the controlled buffer, an unexpected decode failure would cause a panic.Consider either:
- Adding a comment explaining why the unwrap is safe
- Replacing with proper error handling:
- let v = VarInt::decode(&mut cursor).unwrap(); + let v = VarInt::decode(&mut cursor).map_err(|_| VarIntUnexpectedEnd)?;web-transport-proto/src/error.rs (1)
5-18: LGTM! Correct conversion to const fn with GREASE-aware arithmetic.The changes successfully convert both functions to
const fn, enabling compile-time evaluation. The arithmetic on line 11 (code - code / 0x1f) correctly removes GREASE value gaps from the error code range, and the inverse operation on line 17 uses the complementary divisor0x1eto reconstruct the HTTP/3 code.web-transport-quiche/src/h3/mod.rs (1)
1-7: LGTM! Standard module organization.Clean module structure with appropriate re-exports for the h3 subsystem.
web-transport-quiche/src/lib.rs (1)
1-18: LGTM! Well-organized crate root.The module structure clearly separates the public API surface (ez, h3) from internal implementation modules, with selective re-exports providing a clean public interface.
web-transport-proto/src/settings.rs (1)
101-102: LGTM! New error variant added.The
Unsupportederror variant is a useful addition to the error enumeration.web-demo/client.js (2)
64-66: TODO: Partial reads not handled.The comment on line 65 notes that partial reads are not handled. For production use, consider implementing buffering or chunked reading to handle messages that arrive in multiple reads.
Do you want me to generate code that handles partial reads, or is this acceptable for a demo?
22-31: LGTM! Proper certificate pinning setup.The WebTransport connection correctly uses SHA-256 fingerprint-based certificate pinning for local development.
web-transport-quiche/src/ez/client.rs (1)
88-91: Avoid IPv6-only default bindWhen no socket is supplied we bind to
[::]:0. On IPv4-only hosts or environments where IPv6 is disabled, that bind fails withAddrNotAvailable, so every client connection aborts before dialing. Please add an IPv4 fallback (e.g. try"[::]:0"first, then"0.0.0.0:0"if the first bind fails) instead of hard-coding the IPv6 wildcard. raise_major_issueif self.socket.is_none() { - self = self.with_bind("[::]:0")?; + let socket = std::net::UdpSocket::bind("[::]:0").or_else(|err| { + if err.kind() == io::ErrorKind::AddrNotAvailable { + std::net::UdpSocket::bind("0.0.0.0:0") + } else { + Err(err) + } + })?; + self = self.with_socket(socket)?; }web-transport-quiche/src/ez/server.rs (1)
148-148: Track TODO: Error handling across sockets.The TODO indicates that error handling should close all sockets when one errors. This could be important for consistent server shutdown behavior.
Do you want me to open an issue to track implementing coordinated error handling across all socket tasks?
| const params = new URLSearchParams(window.location.search); | ||
|
|
||
| const url = params.get("url") || "https://localhost:4443"; | ||
| const datagram = params.get("datagram") || false; |
There was a problem hiding this comment.
Fix datagram parameter parsing logic.
Line 13 has a logic error: params.get("datagram") returns either a string or null, so || false will evaluate to the string value (e.g., "false") rather than a boolean. Any non-empty string (including "false") is truthy in JavaScript.
Apply this fix:
-const datagram = params.get("datagram") || false;
+const datagram = params.get("datagram") === "true";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const params = new URLSearchParams(window.location.search); | |
| const url = params.get("url") || "https://localhost:4443"; | |
| const datagram = params.get("datagram") || false; | |
| const params = new URLSearchParams(window.location.search); | |
| const url = params.get("url") || "https://localhost:4443"; | |
| const datagram = params.get("datagram") === "true"; |
🤖 Prompt for AI Agents
In web-demo/client.js around lines 10 to 13 the datagram parsing uses
`params.get("datagram") || false` which preserves string values like "false" as
truthy; change the logic to explicitly convert the query value to a boolean,
e.g., replace that line with a comparison that yields true only when the
parameter equals "true" (or use params.has("datagram") if presence-only
semantics are desired), ensuring the result is a real boolean.
| pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> { | ||
| let mut buf = Vec::new(); | ||
| loop { | ||
| stream | ||
| .read_buf(&mut buf) | ||
| .await | ||
| .map_err(|_| CapsuleError::UnexpectedEnd)?; | ||
| let mut limit = std::io::Cursor::new(&buf); | ||
| match Self::decode(&mut limit) { | ||
| Ok(capsule) => return Ok(capsule), | ||
| Err(CapsuleError::UnexpectedEnd) => continue, | ||
| Err(e) => return Err(e), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider limiting buffer growth to prevent memory exhaustion.
The read method uses an unbounded Vec that grows with each iteration until decode succeeds. If the stream continuously sends data without forming a valid capsule, this could lead to excessive memory consumption.
Consider adding a maximum buffer size check:
pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> {
let mut buf = Vec::new();
loop {
+ if buf.len() > MAX_MESSAGE_SIZE {
+ return Err(CapsuleError::MessageTooLong);
+ }
stream
.read_buf(&mut buf)
.awaitNote: This pattern appears in other files (connect.rs, settings.rs) and should be addressed consistently across the codebase.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web-transport-proto/src/capsule.rs around lines 72 to 86, the read method
uses an unbounded Vec that can grow indefinitely; add a maximum buffer size
constant (e.g. MAX_CAPSULE_BUF_BYTES) and enforce it by checking buffer length
before/after each read and returning a CapsuleError::PayloadTooLarge (or a new
suitable error variant) when exceeded; ensure you check prior to continuing the
loop to avoid attempting decode on oversized buffers, and apply the same pattern
to connect.rs and settings.rs for consistency.
web-transport-proto/src/connect.rs
Outdated
| .read_buf(&mut buf) | ||
| .await | ||
| .map_err(|_| ConnectError::UnexpectedEnd)?; | ||
| let mut limit = std::io::Cursor::new(&buf); | ||
| match Self::decode(&mut limit) { | ||
| Ok(response) => return Ok(response), | ||
| Err(ConnectError::UnexpectedEnd) => continue, | ||
| Err(e) => return Err(e), | ||
| } | ||
| } |
There was a problem hiding this comment.
Propagate EOF when reading the response
Same issue on the response side: a zero-length read_buf signals EOF, so the loop should fail fast instead of hanging.(tokio.rs)(tokio.rs)
- stream
- .read_buf(&mut buf)
- .await
- .map_err(|_| ConnectError::UnexpectedEnd)?;
+ let read = stream
+ .read_buf(&mut buf)
+ .await
+ .map_err(|_| ConnectError::UnexpectedEnd)?;
+ if read == 0 {
+ return Err(ConnectError::UnexpectedEnd);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web-transport-proto/src/connect.rs around lines 182 to 191, the loop treats
any Ok read as progress but doesn't handle a zero-length read (EOF), which
causes the loop to hang; change the read handling to capture the number of bytes
read, return Err(ConnectError::UnexpectedEnd) immediately when that number is 0,
and only proceed to construct the Cursor and decode when bytes_read > 0. Ensure
errors from the await are still mapped to ConnectError::UnexpectedEnd and that
the EOF case is detected and propagated before attempting to decode.
| // "recvdrop" in ascii; if you see this then read everything or close(code) | ||
| // decimal: 7305813194079104880 | ||
| const DROP_CODE: u64 = 0x6563766464726F70; |
There was a problem hiding this comment.
Correct DROP_CODE: STOP_SENDING must use a valid HTTP/3 code.
The literal 0x6563766464726F70 evaluates to 7305813194079104880, exceeding the HTTP/3 varint ceiling (< 2^62). Sending STOP_SENDING with this value will fail. Derive the code via web_transport_proto::error_to_http3(...) so it stays in range.
-const DROP_CODE: u64 = 0x6563766464726F70;
+const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x72656376);Committable suggestion skipped: line range outside the PR's diff.
| // "senddrop" in ascii; if you see this then call finish().await or close(code) | ||
| // decimal: 7308889627613622128 | ||
| const DROP_CODE: u64 = 0x656E646464726F70; |
There was a problem hiding this comment.
Fix DROP_CODE: current value is not a valid HTTP/3 error code.
DROP_CODE is set to 0x656E646464726F70, which is 7308889627613622128 in decimal. HTTP/3 error codes are QUIC varints and must be < 2^62; this constant exceeds that bound, so stream_shutdown (and peers) will reject it. Call web_transport_proto::error_to_http3(...) with a valid 32-bit WebTransport code instead of hard-coding the raw 64-bit literal.
-const DROP_CODE: u64 = 0x656E646464726F70;
+const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x73656E64);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web-transport-quiche/src/ez/send.rs around lines 17 to 19, the DROP_CODE
constant is an invalid HTTP/3/QUIC varint (0x656E646464726F70 > 2^62) and will
be rejected; replace the hard-coded 64-bit literal with a call to
web_transport_proto::error_to_http3(...) using a valid 32-bit WebTransport error
code (e.g., a defined WebTransport code or custom u32), remove the literal
DROP_CODE constant, and use the produced HTTP/3 error value when calling
stream_shutdown so the code is a valid QUIC varint.
| #[error("connection error: {0}")] | ||
| ConnectionError(#[from] quinn::ConnectionError), | ||
| Connection(quinn::ConnectionError), | ||
|
|
||
| #[error("webtransport error: {0}")] | ||
| WebTransportError(#[from] WebTransportError), | ||
| WebTransport(#[from] WebTransportError), | ||
|
|
||
| #[error("send datagram error: {0}")] | ||
| SendDatagramError(#[from] quinn::SendDatagramError), | ||
| #[error("datagram error: {0}")] | ||
| Datagram(#[from] quinn::SendDatagramError), | ||
| } |
There was a problem hiding this comment.
Add conversion for received datagram errors
Session::read_datagram() now calls .map_err(SessionError::from), but SessionError only implements From<quinn::SendDatagramError>. Without a From<quinn::ReadDatagramError> variant, this no longer compiles. Please extend the enum so that incoming datagram failures map cleanly (e.g., add a quinn::ReadDatagramError variant), which lets the new call site type-check.
#[error("datagram error: {0}")]
Datagram(#[from] quinn::SendDatagramError),
+
+ #[error("datagram error: {0}")]
+ DatagramRecv(#[from] quinn::ReadDatagramError),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[error("connection error: {0}")] | |
| ConnectionError(#[from] quinn::ConnectionError), | |
| Connection(quinn::ConnectionError), | |
| #[error("webtransport error: {0}")] | |
| WebTransportError(#[from] WebTransportError), | |
| WebTransport(#[from] WebTransportError), | |
| #[error("send datagram error: {0}")] | |
| SendDatagramError(#[from] quinn::SendDatagramError), | |
| #[error("datagram error: {0}")] | |
| Datagram(#[from] quinn::SendDatagramError), | |
| } | |
| #[error("connection error: {0}")] | |
| Connection(quinn::ConnectionError), | |
| #[error("webtransport error: {0}")] | |
| WebTransport(#[from] WebTransportError), | |
| #[error("datagram error: {0}")] | |
| Datagram(#[from] quinn::SendDatagramError), | |
| #[error("datagram error: {0}")] | |
| DatagramRecv(#[from] quinn::ReadDatagramError), | |
| } |
🤖 Prompt for AI Agents
In web-transport-quinn/src/error.rs around lines 42 to 50, the SessionError enum
lacks a variant to convert quinn::ReadDatagramError so
Session::read_datagram().map_err(SessionError::from) fails to compile; add a new
enum variant like #[error("read datagram error: {0}")] ReadDatagram(#[from]
quinn::ReadDatagramError) (placed alongside the existing
Datagram/SendDatagramError variant) so incoming datagram read failures are
converted cleanly via From; ensure quinn::ReadDatagramError is in scope and the
error message follows the existing pattern.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
web-transport-proto/src/capsule.rs (1)
74-88: BoundCapsule::read’s buffer to avoid unboundedVecgrowth
Capsule::readuses an ever-growingVec:let mut buf = Vec::new(); loop { if stream.read_buf(&mut buf).await? == 0 { return Err(CapsuleError::UnexpectedEnd); } let mut limit = std::io::Cursor::new(&buf); match Self::decode(&mut limit) { ... } }If a peer keeps sending data that never forms a valid capsule,
bufcan grow without limit and cause memory pressure.MAX_MESSAGE_SIZEconstrains the decoded payload, but not the temporary buffer backing the cursor.Consider enforcing a max buffer size and failing fast once exceeded, e.g. with a separate
MAX_CAPSULE_BUF_BYTESconstant:- pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> { - let mut buf = Vec::new(); + pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> { + let mut buf = Vec::new(); loop { + // Prevent unbounded growth on malformed or malicious input. + if buf.len() >= MAX_CAPSULE_BUF_BYTES { + return Err(CapsuleError::MessageTooLong); + } if stream.read_buf(&mut buf).await? == 0 { return Err(CapsuleError::UnexpectedEnd); } ... } }(with
MAX_CAPSULE_BUF_BYTESsized appropriately for your use case).web-transport-quiche/src/ez/recv.rs (1)
20-22: Fix DROP_CODE: invalid HTTP/3 STOP_SENDING value.0x6563766464726F70 equals 7 305 813 194 079 104 880, which is above the QUIC/HTTP3 varint ceiling (2^62 − 1 ≈ 4 611 686 018 427 387 903). When
RecvStream::dropasks the driver to send STOP_SENDING with this code,stream_shutdownrejects it and the cleanup path fails, leaking the stream. Useweb_transport_proto::error_to_http3(e.g. with the 32-bit “recv” code) so the constant always stays within the valid range.Apply this diff:
-const DROP_CODE: u64 = 0x6563766464726F70; +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x72656376);web-transport-quiche/src/ez/send.rs (1)
19-21: Fix DROP_CODE: invalid HTTP/3 RESET_STREAM value.0x656E646464726F70 expands to 7 308 889 627 613 622 128, which also exceeds the QUIC/HTTP3 varint limit (< 2^62). When the drop path emits RESET_STREAM with this code, quiche rejects it and the stream never closes cleanly. Reuse
web_transport_proto::error_to_http3to derive a compliant value (e.g. the 32-bit “send” code) before callingstream_shutdown.Apply this diff:
-const DROP_CODE: u64 = 0x656E646464726F70; +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x73656E64);web-transport-quiche/src/connection.rs (1)
105-125: Session closure still closes entire QUIC connection (duplicate concern).The TODO on line 111 correctly identifies that
self.close(code, &reason)closes the entire QUIC connection rather than just the WebTransport session. This prevents multiplexing multiple sessions over one connection.
🧹 Nitpick comments (12)
web-transport-quiche/README.md (1)
5-7: Refine informal language and reduce exclamation marks.The description uses informal phrasing ("annoying API") and has excessive exclamation marks. Consider a more professional tone:
- A wrapper around the Quiche, abstracting away the annoying API and HTTP/3 internals. - Provides a QUIC-like API but with web support! + A wrapper around Quiche that abstracts away the HTTP/3 internals. + Provides a QUIC-like API with WebTransport support.web-transport-quiche/examples/echo-server.rs (3)
35-45: Consider PathBuf::display() for better path handling.The current approach using
to_str()will fail for paths containing non-UTF8 sequences. While this is rare, usingPathBuf::display()would handle such cases more gracefully. However, if the underlying TLS API requires&str, the current approach with contextual error handling is acceptable.Consider this alternative if the API supports it:
let tls = web_transport_quiche::ez::CertificatePath { - cert: args - .tls_cert - .to_str() - .context("failed to convert path to str")?, - private_key: args - .tls_key - .to_str() - .context("failed to convert path to str")?, + cert: &args.tls_cert.display().to_string(), + private_key: &args.tls_key.display().to_string(), kind: web_transport_quiche::ez::CertificateKind::X509, };
77-92: Consider distinguishing normal session closure from errors.The current loop treats all
accept_bi()errors as abnormal (line 60 logs "connection closed: {err}"). When a session closes gracefully, this may log a benign error as if it were a problem.You could distinguish between normal closure and actual errors by matching on the error type:
loop { match session.accept_bi().await { Ok((mut send, mut recv)) => { tracing::info!("accepted stream"); let mut msg: Bytes = recv.read_all(1024).await?; tracing::info!("recv: {}", String::from_utf8_lossy(&msg)); tracing::info!("send: {}", String::from_utf8_lossy(&msg)); send.write_buf_all(&mut msg).await?; send.finish()?; tracing::info!("echo successful!"); } Err(e) => { // Check if this is a normal closure vs an actual error tracing::debug!("accept_bi failed: {e}"); break; } } }
80-80: TODO: Datagram support.The TODO comment indicates that datagram handling is not yet implemented in this example.
Would you like me to generate code to handle datagrams alongside bidirectional streams, or open a new issue to track this enhancement?
web-transport-trait/src/lib.rs (2)
194-205: Document safety invariants aroundRecvStream::read_buf’stransmute
read_bufuses:let dst = unsafe { std::mem::transmute::<&mut bytes::buf::UninitSlice, &mut [u8]>(buf.chunk_mut()) };as a default implementation on the trait. This relies on all
RecvStream::readimpls treatingdstas a write-only buffer and only initializing the firstsizebytes before we calladvance_mut(size). That’s subtle for downstream implementors.I’d strongly suggest adding an explicit SAFETY comment here spelling out the required invariants (no reads from
dstbefore it’s fully written, no writes past its length), and ideally referencing thebytesrecommended pattern, so future refactors don’t accidentally violate this assumption.A minimal change:
- let dst = unsafe { - std::mem::transmute::<&mut bytes::buf::UninitSlice, &mut [u8]>(buf.chunk_mut()) - }; + // SAFETY: `dst` points to uninitialized capacity from `buf`. Implementations of + // `RecvStream::read` must treat it as a write-only output buffer, only initialize + // the first `size` bytes, and never read from or write past `dst.len()`. We then + // call `advance_mut(size)` to mark those bytes as initialized. + let dst = unsafe { + std::mem::transmute::<&mut bytes::buf::UninitSlice, &mut [u8]>(buf.chunk_mut()) + };
177-183: AdjustRecvStream::readdoc comment to match behaviorThe doc for
RecvStream::readsays “This returns a chunk of data instead of copying”, but the signature takes&mut [u8]and returns a length, implying data is copied into the caller’s buffer.Might be clearer to rephrase along the lines of “Read into the provided buffer, returning the number of bytes read (or None on EOF).”
web-transport-proto/src/connect.rs (1)
110-124: Limit buffer growth in ConnectRequest/ConnectResponsereadloopsBoth
ConnectRequest::readandConnectResponse::readuse an unboundedVec:let mut buf = Vec::new(); loop { if stream.read_buf(&mut buf).await? == 0 { return Err(ConnectError::UnexpectedEnd); } let mut limit = std::io::Cursor::new(&buf); match Self::decode(&mut limit) { ... } }If a peer sends a very large or never-ending header block that never decodes successfully,
bufwill grow indefinitely, which can become a memory DoS.Consider enforcing a maximum size for
buf(e.g.MAX_CONNECT_BUF_BYTES) and returningUnexpectedEndor a dedicated “too large” error once that limit is exceeded, similar to the capsule/settings protections.Also applies to: 184-198
web-transport-quiche/src/error.rs (1)
28-38: Fix minorStreamErrordisplay string typosThere are a few small issues in the
StreamErrorerror messages:#[error("reset stream: {0})")] Reset(u32), #[error("stop stream: {0})")] Stop(u32), #[error("invalid reset code: {0}")] InvalidStop(u64),
- Both
ResetandStopmessages have an extra closing)at the end.InvalidStoplikely wants"invalid stop code: {0}"instead of"invalid reset code".Suggested fix:
- #[error("reset stream: {0})")] + #[error("reset stream: {0}")] Reset(u32), - #[error("stop stream: {0})")] + #[error("stop stream: {0}")] Stop(u32), - #[error("invalid reset code: {0}")] + #[error("invalid stop code: {0}")] InvalidStop(u64),web-transport-quiche/src/recv.rs (2)
50-57: Clarify non-zero STOP code used onDropOn drop you emit a STOP_SENDING with a special “RECV” code:
if !self.inner.is_closed() { tracing::warn!("stream dropped without `close` or reading all contents"); self.inner.close(DROP_CODE) }The trait-level docs in
web-transport-traitcurrently say “The stream will be closed with STOP_SENDING code=0 when dropped.” This implementation intentionally uses a non-zero diagnostic code.Either aligning the code with the trait doc (code 0) or explicitly calling out in the docs that quiche uses a non-zero STOP code for debugging/diagnostics would make the behavior less surprising for callers.
70-88: Consider overriding traitread_bufto reuse the inner implementation
RecvStreamexposes an inherent:pub async fn read_buf<B: BufMut>(&mut self, buf: &mut B) -> Result<Option<usize>, StreamError> { self.inner.read_buf(buf).await.map_err(Into::into) }but the
web_transport_trait::RecvStreamimpl only overridesreadandread_chunk. When used via the trait,read_bufwill go through the generic default implementation (with its own unsafechunk_mutlogic) instead of this specialized path.If you want quiche-based streams to benefit from the inner
read_bufbehavior when accessed through the trait, you could overrideread_bufin the trait impl to delegate to the inherent method, similar toreadandread_chunk.web-transport-quiche/src/ez/server.rs (1)
143-147: Address TODO: coordinate shutdown when a socket task fails.The TODO on line 145 identifies that when one socket task errors, the others should be closed. Currently, socket tasks run independently and a failure in one doesn't affect others, potentially leaving the server in an inconsistent state.
Do you want me to generate an implementation that uses a shared cancellation token or error channel to coordinate shutdown across all socket tasks when one fails?
web-transport-quiche/src/ez/driver.rs (1)
215-244: Replace assertions with defensive error handling.Lines 224 and 233 use
assert!(waker.is_none())assuming newly accepted streams won't have pending wakers. If this assumption is violated (e.g., due to changes in quiche behavior), the server will panic. Consider defensive handling instead.let mut state = RecvState::new(stream_id); let waker = state.flush(qconn)?; -assert!(waker.is_none()); +if let Some(waker) = waker { + tracing::warn!(?stream_id, "unexpected waker on newly accepted stream"); + waker.wake(); +}Apply similar changes to line 233 and in
accept_uni(lines 254-255).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
web-transport-proto/src/capsule.rs(5 hunks)web-transport-proto/src/connect.rs(6 hunks)web-transport-proto/src/settings.rs(4 hunks)web-transport-quiche/README.md(1 hunks)web-transport-quiche/examples/echo-client.rs(1 hunks)web-transport-quiche/examples/echo-server.rs(1 hunks)web-transport-quiche/src/connection.rs(1 hunks)web-transport-quiche/src/error.rs(1 hunks)web-transport-quiche/src/ez/client.rs(1 hunks)web-transport-quiche/src/ez/connection.rs(1 hunks)web-transport-quiche/src/ez/driver.rs(1 hunks)web-transport-quiche/src/ez/lock.rs(1 hunks)web-transport-quiche/src/ez/recv.rs(1 hunks)web-transport-quiche/src/ez/send.rs(1 hunks)web-transport-quiche/src/ez/server.rs(1 hunks)web-transport-quiche/src/h3/connect.rs(1 hunks)web-transport-quiche/src/recv.rs(1 hunks)web-transport-quinn/src/error.rs(2 hunks)web-transport-trait/src/lib.rs(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- web-transport-proto/src/settings.rs
- web-transport-quiche/examples/echo-client.rs
- web-transport-quiche/src/ez/lock.rs
- web-transport-quiche/src/ez/connection.rs
🧰 Additional context used
🧬 Code graph analysis (14)
web-transport-quiche/src/h3/connect.rs (2)
web-transport-quiche/src/h3/request.rs (2)
url(30-32)accept(14-27)web-transport-proto/src/connect.rs (2)
read(110-124)read(184-198)
web-transport-quiche/src/error.rs (2)
web-transport-proto/src/error.rs (1)
error_from_http3(5-14)web-transport-trait/src/lib.rs (2)
session_error(15-15)stream_error(18-20)
web-transport-quiche/src/recv.rs (3)
web-transport-quiche/src/ez/recv.rs (12)
std(149-149)std(268-268)new(56-69)new(221-223)read(229-234)read_chunk(236-238)read_buf(265-278)read_all(280-285)close(288-295)closed(327-329)drop(333-346)poll_read(350-361)web-transport-trait/src/lib.rs (11)
std(195-195)read(180-183)read_chunk(211-222)read_buf(189-206)read_all(239-245)close(67-67)close(155-155)close(228-228)closed(70-70)closed(167-167)closed(236-236)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-proto/src/connect.rs (3)
web-transport-proto/src/capsule.rs (4)
from(164-166)read(74-88)decode(22-72)write(124-129)web-transport-proto/src/settings.rs (5)
from(107-109)read(142-159)decode(19-21)decode(117-140)write(177-183)web-transport-proto/src/varint.rs (7)
from(70-72)from(76-78)from(82-84)from(88-90)read(167-189)decode(130-164)write(206-223)
web-transport-quiche/src/ez/send.rs (2)
web-transport-quiche/src/ez/recv.rs (13)
std(149-149)std(268-268)new(56-69)new(221-223)id(225-227)poll_closed(107-118)poll_closed(307-317)flush(120-207)is_closed(209-211)is_closed(303-305)close(288-295)closed(327-329)drop(333-346)web-transport-quiche/src/ez/driver.rs (7)
new(35-54)new(152-165)new(551-558)is_closed(64-66)write(268-296)close(56-58)closed(60-62)
web-transport-quinn/src/error.rs (3)
web-transport-quiche/src/error.rs (5)
from(45-57)from(61-74)session_error(78-84)session_error(94-100)stream_error(86-91)web-transport-trait/src/lib.rs (2)
session_error(15-15)stream_error(18-20)web-transport-proto/src/error.rs (1)
error_from_http3(5-14)
web-transport-proto/src/capsule.rs (3)
web-transport-proto/src/connect.rs (7)
read(110-124)read(184-198)decode(69-108)decode(162-182)write(148-153)write(215-220)from(58-60)web-transport-proto/src/settings.rs (5)
read(142-159)decode(19-21)decode(117-140)write(177-183)from(107-109)web-transport-proto/src/varint.rs (3)
read(167-189)decode(130-164)write(206-223)
web-transport-quiche/examples/echo-server.rs (4)
web-transport-quiche/examples/echo-client.rs (1)
main(17-60)web-transport-quiche/src/connection.rs (3)
new(63-102)new(361-383)url(288-290)web-transport-quiche/src/h3/connect.rs (1)
url(96-98)web-transport-quiche/src/h3/request.rs (1)
url(30-32)
web-transport-quiche/src/ez/client.rs (4)
web-transport-quiche/src/client.rs (2)
default(30-32)connect(73-80)web-transport-quiche/src/ez/connection.rs (4)
new(81-83)new(123-138)accept_bi(141-146)accept_uni(149-154)web-transport-quiche/src/ez/driver.rs (5)
new(35-54)new(152-165)new(551-558)accept_bi(215-244)accept_uni(246-266)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)
web-transport-trait/src/lib.rs (15)
web-transport-quiche/src/error.rs (3)
session_error(78-84)session_error(94-100)stream_error(86-91)web-transport-quinn/src/error.rs (5)
session_error(266-273)session_error(277-283)session_error(294-300)stream_error(285-290)stream_error(302-307)web-transport-ws/src/error.rs (2)
session_error(52-61)stream_error(63-69)web-transport-quiche/src/connection.rs (4)
closed(265-267)closed(330-332)close(254-262)close(326-328)web-transport-quiche/src/ez/connection.rs (3)
closed(193-195)close(85-91)close(184-187)web-transport-quiche/src/ez/driver.rs (2)
closed(60-62)close(56-58)web-transport-quiche/src/ez/recv.rs (2)
closed(327-329)close(288-295)web-transport-quiche/src/ez/send.rs (4)
closed(350-352)set_priority(354-361)finish(282-302)close(310-317)web-transport-quiche/src/recv.rs (4)
closed(45-47)closed(86-88)close(41-43)close(82-84)web-transport-quinn/src/session.rs (4)
closed(261-263)closed(524-526)close(248-258)close(520-522)web-transport-wasm/src/recv.rs (1)
closed(74-88)web-transport-ws/src/session.rs (8)
closed(466-473)closed(610-621)closed(725-757)set_priority(569-571)finish(585-608)close(450-464)close(573-583)close(717-723)web-transport-wasm/src/session.rs (2)
closed(102-104)close(94-99)web-transport-wasm/src/send.rs (3)
closed(59-73)set_priority(53-56)finish(45-48)web-transport-quiche/src/send.rs (5)
set_priority(46-48)set_priority(101-103)finish(42-44)finish(109-111)close(50-53)
web-transport-quiche/src/connection.rs (5)
web-transport-quiche/src/h3/connect.rs (4)
url(96-98)session_id(91-93)accept(38-52)open(63-88)web-transport-quinn/src/session.rs (19)
url(295-297)connect(116-128)run_closed(96-112)accept_uni(131-141)accept_uni(504-506)accept_bi(144-154)accept_bi(508-510)open_uni(157-169)open_uni(516-518)open_bi(172-184)open_bi(512-514)close(248-258)closed(261-263)closed(524-526)raw(282-293)send_datagram(219-235)send_datagram(528-530)poll_accept_uni(375-414)poll_accept_bi(441-471)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)web-transport-quiche/src/ez/connection.rs (9)
new(81-83)new(123-138)accept_uni(149-154)accept_bi(141-146)open_uni(170-178)open_bi(157-167)close(85-91)close(184-187)closed(193-195)web-transport-quiche/src/h3/settings.rs (3)
connect(36-43)accept(45-56)open(58-68)
web-transport-quiche/src/ez/server.rs (4)
web-transport-quiche/src/server.rs (16)
ServerBuilder(46-46)ServerBuilder(53-53)ServerBuilder(62-62)default(32-34)new(38-40)new(105-110)with_listener(42-47)with_listener(73-75)with_socket(49-56)with_socket(77-79)with_bind(58-65)with_bind(81-83)with_settings(67-69)with_settings(85-87)with_cert(91-93)accept(113-126)web-transport-quiche/src/ez/driver.rs (5)
new(35-54)new(152-165)new(551-558)accept_bi(215-244)accept_uni(246-266)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)web-transport-quiche/src/ez/connection.rs (2)
accept_bi(141-146)accept_uni(149-154)
web-transport-quiche/src/ez/driver.rs (4)
web-transport-quiche/src/ez/recv.rs (6)
new(56-69)new(221-223)close(288-295)closed(327-329)id(225-227)drop(333-346)web-transport-quiche/src/ez/connection.rs (12)
new(81-83)new(123-138)close(85-91)close(184-187)closed(193-195)open_bi(157-167)open_uni(170-178)accept_bi(141-146)accept_uni(149-154)drop(103-105)wait(93-95)poll(59-68)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)web-transport-quiche/src/ez/send.rs (6)
new(53-65)new(216-218)close(310-317)closed(350-352)id(220-222)drop(365-378)
web-transport-quiche/src/ez/recv.rs (3)
web-transport-trait/src/lib.rs (9)
std(195-195)read(180-183)read_buf(189-206)close(67-67)close(155-155)close(228-228)closed(70-70)closed(167-167)closed(236-236)web-transport-quiche/src/ez/driver.rs (4)
is_closed(64-66)read(179-213)close(56-58)closed(60-62)web-transport-quiche/src/ez/send.rs (7)
id(220-222)flush(114-192)is_closed(204-206)is_closed(325-327)close(310-317)closed(350-352)drop(365-378)
🪛 LanguageTool
web-transport-quiche/README.md
[style] ~7-~7: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1549 characters long)
Context: ...des a QUIC-like API but with web support! ## WebTransport [WebTransport](https:/...
(EN_EXCESSIVE_EXCLAMATION)
⏰ 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 (7)
web-transport-proto/src/capsule.rs (1)
124-129: Io error variant + async read/write wiring looks consistentThe new
CapsuleError::Io(Arc<std::io::Error>)plusimpl From<std::io::Error> for CapsuleErrorcleanly propagate underlying I/O errors fromread_buf/write_all_buf, whileUnexpectedEndremains reserved for truncated capsules/EOF.The
read/writehelpers themselves are straightforward wrappers arounddecode/encodeand match the patterns used inconnect.rsandsettings.rs, so the API surface is consistent.Also applies to: 159-167
web-transport-proto/src/connect.rs (1)
53-61: Io error variant andFrom<std::io::Error>mapping look goodAdding
ConnectError::Io(Arc<std::io::Error>)and usingFrom<std::io::Error>means:
- real I/O failures are preserved instead of being collapsed into
UnexpectedEnd- the new async
read/writehelpers can use?ergonomically.This matches the pattern used elsewhere in
web-transport-protoand keeps error reporting precise.web-transport-quiche/src/ez/server.rs (1)
156-179: LGTM: Clean connection acceptance loop.The socket task correctly handles connection acceptance, driver initialization, and graceful shutdown when the accept channel closes. Returning
Ok(())on send failure is appropriate as it indicates the server is shutting down.web-transport-quiche/src/ez/driver.rs (3)
471-474: Verifyshould_act()implementation.The method unconditionally returns
truewith a TODO comment. This may impact performance if the driver attempts work when there's nothing to do. Verify whether the current implementation is acceptable or if proper act detection logic should be added before production use.
338-379: LGTM: Proper lock handling and waker management.The critical section correctly holds the driver lock only during state updates (lines 338-370), then releases it before waking tasks (lines 373-379). This prevents deadlocks and minimizes contention.
507-540: LGTM: Comprehensive connection close handling.The method correctly determines the connection error from multiple sources with appropriate priority, then wakes all blocked tasks. The error cascading logic (local error → local close → peer close → connection result → unknown) ensures proper error propagation.
web-transport-quiche/src/connection.rs (1)
169-189: No changes needed — quiche automatically ensures headers are sent before body bytes on the same stream.Quiche's HTTP/3 API sends HEADERS frames as the initial response/request and will queue HEADERS before sending body bytes on that stream. Priority only affects scheduling between streams, not within a stream. The current implementation is correct and does not require explicit priority handling like the Quinn version.
| tracing::info!("accepted stream"); | ||
|
|
||
| // Read the message and echo it back. | ||
| let mut msg: Bytes = recv.read_all(1024).await?; |
There was a problem hiding this comment.
Hardcoded 1024-byte message limit may cause truncation.
The read_all(1024) call imposes a 1024-byte limit on messages. Messages exceeding this size will be truncated or trigger an error, which could be confusing for users testing the echo server with larger payloads.
Consider making this configurable or using a larger default:
-let mut msg: Bytes = recv.read_all(1024).await?;
+let mut msg: Bytes = recv.read_all(64 * 1024).await?; // 64 KB limitOr add a CLI argument to configure the maximum message size.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut msg: Bytes = recv.read_all(1024).await?; | |
| let mut msg: Bytes = recv.read_all(64 * 1024).await?; // 64 KB limit |
🤖 Prompt for AI Agents
In web-transport-quiche/examples/echo-server.rs around line 84, the call
read_all(1024) hardcodes a 1024-byte maximum which can truncate larger messages;
replace the literal with a configurable max_message_size (e.g., a const
DEFAULT_MAX: usize = 64*1024) and use a command-line flag or env var to override
it, parse that value at startup into max_message_size, and pass that variable to
recv.read_all(max_message_size). Ensure the default is larger (or unlimited if
supported) and validate the provided size for sane bounds.
| # Usage | ||
| To use web-transport-quiche, figure it out yourself lul. |
There was a problem hiding this comment.
Add actual usage documentation to the README.
The Usage section is incomplete and provides no guidance to users. A proper README should include examples of how to initialize a client/server, create streams, send data, etc.
Based on the AI summary mentioning echo-server.rs and echo-client.rs examples, consider adding:
- Quick start guide (client/server setup)
- Basic stream usage example
- Datagram usage example
- Link to full examples in the examples/ directory
🤖 Prompt for AI Agents
In web-transport-quiche/README.md around lines 30 to 31, the Usage section is
placeholder text and lacks any guidance; replace it with a concise Usage section
that includes a quick start showing how to initialize a server and a client
(referencing echo-server.rs and echo-client.rs), a short example of
creating/accepting streams and sending/receiving data, a brief datagram
send/receive snippet, and a link to the examples/ directory for full working
examples; keep examples minimal, language-agnostic descriptions or pseudocode
references to the example files, and include commands to build/run the example
server and client.
| fn send_datagram(&self, _payload: bytes::Bytes) -> Result<(), Self::Error> { | ||
| todo!() | ||
| } | ||
|
|
||
| async fn recv_datagram(&self) -> Result<bytes::Bytes, SessionError> { | ||
| todo!() | ||
| } | ||
|
|
||
| fn max_datagram_size(&self) -> usize { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Replace todo!() with proper error handling or implementation.
The datagram methods in the Session trait implementation all panic with todo!(). Lines 191-251 contain a commented-out implementation. Either complete the implementation, return a proper error indicating datagrams are unsupported, or document why these methods are incomplete.
Consider this approach:
fn send_datagram(&self, _payload: bytes::Bytes) -> Result<(), Self::Error> {
- todo!()
+ Err(SessionError::Unknown) // or add a specific DatagramUnsupported variant
}
async fn recv_datagram(&self) -> Result<bytes::Bytes, SessionError> {
- todo!()
+ Err(SessionError::Unknown)
}
fn max_datagram_size(&self) -> usize {
- todo!()
+ 0
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web-transport-quiche/src/connection.rs around lines 314 to 324, the Session
trait datagram methods currently call todo!() and must be replaced: either
restore and adapt the commented-out implementation between lines 191–251 into
these methods (porting the logic, error mapping, and required state accesses),
or explicitly return a clear non-panicking error indicating datagrams are
unsupported (e.g., make send_datagram and recv_datagram return
Err(SessionError::UnsupportedDatagrams) or an equivalent SessionError variant,
and have max_datagram_size return 0) so callers receive a proper error instead
of panics; pick one approach and implement it consistently, updating any
docs/comments accordingly.
| self.capacity -= n; | ||
|
|
There was a problem hiding this comment.
Prevent capacity underflow while flushing.
poll_write_buf already debits self.capacity by the queued byte count, so as soon as we fill the allowance self.capacity is zero. On the first successful stream_send, self.capacity -= n; subtracts a positive n from zero, panicking in debug builds and wrapping to a bogus huge value in release until the later reassignment happens. Switch to a saturating subtraction (or remove the decrement) so the counter never underflows.
Apply this diff:
- self.capacity -= n;
+ self.capacity = self.capacity.saturating_sub(n);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.capacity -= n; | |
| self.capacity = self.capacity.saturating_sub(n); | |
🤖 Prompt for AI Agents
In web-transport-quiche/src/ez/send.rs around lines 151 to 152, the direct
subtraction self.capacity -= n can underflow; replace that decrement with a
saturating subtraction (or remove the decrement entirely if capacity was already
debited earlier) so the capacity counter never underflows—i.e., ensure capacity
is updated using a saturating operation or skip the extra decrement.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
web-transport-quiche/src/ez/send.rs (2)
19-21: Fix DROP_CODE: current value exceeds HTTP/3 varint limit.
DROP_CODEis set to0x656E646464726F70(7308889627613622128 in decimal), which exceeds the QUIC varint maximum of 2^62 - 1. This will causestream_shutdownto fail. Useweb_transport_proto::error_to_http3(...)with a valid 32-bit WebTransport code instead.Apply this diff:
-// "senddrop" in ascii; if you see this then call finish().await or close(code) -// decimal: 7308889627613622128 -const DROP_CODE: u64 = 0x656E646464726F70; +// "send" in ascii; if you see this then call finish().await or close(code) +// hex: 0x73656E64, or 0x52E51B4DCE20 as an HTTP error code +// decimal: 1685221232, or 91143959072288 as an HTTP error code +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x73656E64);
114-192: Prevent capacity underflow in flush loop.Line 151 performs
self.capacity -= nafterstream_send, butself.capacityis already decremented inpoll_write_buf(line 92). When a partial write occurs, this double-decrement can cause underflow, panicking in debug builds or wrapping to a huge value in release builds.Apply this diff:
- self.capacity -= n; + self.capacity = self.capacity.saturating_sub(n);web-transport-quiche/src/connection.rs (2)
110-130: Session closure incorrectly closes the entire QUIC connection.Line 118 calls
self.close(code, &reason), which closes the entire QUIC connection rather than just the WebTransport session. This prevents multiplexing multiple sessions over one connection. The TODO comment on line 116 correctly identifies this issue.Consider returning the close event to the application instead:
async fn run_closed(self, connect: h3::Connect) -> (u32, String) { let (_send, mut recv) = connect.into_inner(); loop { match web_transport_proto::Capsule::read(&mut recv).await { Ok(web_transport_proto::Capsule::CloseWebTransportSession { code, reason }) => { return (code, reason); } Ok(web_transport_proto::Capsule::Unknown { typ, payload }) => { tracing::warn!("unknown capsule: type={typ} size={}", payload.len()); } Err(_) => { return (500, "capsule error".to_string()); } } } }
337-347: Replacetodo!()with proper implementation or error.The datagram methods panic with
todo!(). Either implement them (there's commented code at lines 209-269 that could be adapted) or return a proper error indicating datagrams are unsupported.Consider one of these approaches:
Option 1: Return an error
fn send_datagram(&self, _payload: bytes::Bytes) -> Result<(), Self::Error> { - todo!() + Err(SessionError::Unknown) // or add DatagramUnsupported variant } async fn recv_datagram(&self) -> Result<bytes::Bytes, SessionError> { - todo!() + Err(SessionError::Unknown) } fn max_datagram_size(&self) -> usize { - todo!() + 0 }Option 2: Restore the commented implementation (lines 209-269) and adapt it to the current API.
web-transport-quiche/src/ez/recv.rs (1)
20-22: Fix DROP_CODE: current value exceeds HTTP/3 varint limit.
DROP_CODEis set to0x6563766464726F70(7305813194079104880 in decimal), which exceeds the QUIC varint maximum of 2^62 - 1. Sending STOP_SENDING with this value will fail. Useweb_transport_proto::error_to_http3(...)with a valid 32-bit code.Apply this diff:
-// "recvdrop" in ascii; if you see this then read everything or close(code) -// decimal: 7305813194079104880 -const DROP_CODE: u64 = 0x6563766464726F70; +// "recv" in ascii; if you see this then read everything or close(code) +// hex: 0x72656376, or 0x52E4EA9B7F80 as an HTTP error code +// decimal: 1919120246, or 91143142080384 as an HTTP error code +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x72656376);
🧹 Nitpick comments (7)
flake.nix (1)
40-42: LGTM! shellHook correctly sets up runtime library path.The shellHook appropriately configures LD_LIBRARY_PATH for native dependencies. The approach is standard for Nix development environments.
For slightly more robust handling when
LD_LIBRARY_PATHis unset, consider this pattern:- shellHook = '' - export LD_LIBRARY_PATH=${pkgs.lib.makeLibraryPath [pkgs.stdenv.cc.cc.lib]}:$LD_LIBRARY_PATH - ''; + shellHook = '' + export LD_LIBRARY_PATH=${pkgs.lib.makeLibraryPath [pkgs.stdenv.cc.cc.lib]}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} + '';This avoids a trailing colon when the variable is unset, though the current implementation works fine in practice.
web-transport-quiche/src/ez/server.rs (1)
142-201: Socket task errors are silently dropped; consider surfacing them
run_socketreturnsio::Result<()>, but theJoinSetis never polled, so anyErrfrom the per‑socket task is effectively ignored. That can make handshake/socket failures invisible to callers and just stop new accepts from that socket. Consider either:
- Logging inside
run_socketbefore returning an error, or- Having a supervisor that periodically
join_next()s fromtasksand logs/handles failures (possibly shutting down remaining sockets).This would improve observability and debuggability without changing the public API.
web-transport-quiche/src/h3/connect.rs (1)
101-103: Consider replacingunwrap()withexpect()and a message.The conversion from stream ID to
VarIntshould succeed since QUIC stream IDs are varints, but adding anexpect()message would make debugging easier if this assumption is violated.Apply this diff:
pub fn session_id(&self) -> VarInt { - VarInt::try_from(u64::from(self.send.id())).unwrap() + VarInt::try_from(u64::from(self.send.id())) + .expect("stream ID must be a valid varint") }web-transport-quiche/src/ez/recv.rs (1)
147-152: Document safety invariants forunsafetransmute.The
unsafetransmute fromMaybeUninit<u8>to&mut [u8]avoids zeroing for performance, but relies onstream_recvinitializing all bytes it claims to read. Adding a safety comment would clarify this invariant.+ // SAFETY: stream_recv will initialize all bytes it claims to have read (returned in `n`). + // We immediately call set_len after reading to mark only the initialized portion as valid. let buf: &mut [u8] = unsafe {web-transport-quiche/src/ez/driver.rs (2)
212-263: Consider defensive handling instead of assertions.Lines 221, 230, and 252 use
assert!(waker.is_none())after flushing newly created stream states. While these assertions should be safe (newly created states have no blocked operations), they're fragile and will panic if the assumption is ever violated.Consider replacing with logged warnings:
let mut state = RecvState::new(stream_id); let waker = state.flush(qconn)?; - assert!(waker.is_none()); + if let Some(waker) = waker { + tracing::warn!(?stream_id, "unexpected waker on new recv stream"); + waker.wake(); + }Apply similar changes at lines 230 and 252.
468-471: Address or document the TODO.The
should_actmethod always returnstrue, which may cause unnecessary wakeups and reduce efficiency. Consider implementing a proper check for pending work.For example:
fn should_act(&self) -> bool { let driver = self.state.lock(); !driver.send.is_empty() || !driver.recv.is_empty() || !driver.bi.create.is_empty() || !driver.uni.create.is_empty() || driver.is_closed() }Alternatively, if the current behavior is intentional for this initial implementation, document why always returning
trueis acceptable.web-transport-quinn/src/crypto.rs (1)
24-44: Consider improving the panic message clarity.The current panic message doesn't distinguish between the case where both features are enabled versus neither feature is enabled. When debugging, developers would benefit from a more specific error message.
For example:
#[allow(unreachable_code)] { + #[cfg(all(feature = "aws-lc-rs", feature = "ring"))] + panic!("Both 'ring' and 'aws-lc-rs' features are enabled. Enable only one."); + + #[cfg(not(any(feature = "aws-lc-rs", feature = "ring")))] + panic!("No crypto provider available. Enable either 'ring' or 'aws-lc-rs' feature, or call CryptoProvider::set_default()."); + panic!( - "CryptoProvider::set_default() must be called; or only enable one ring/aws-lc-rs feature." + "CryptoProvider::set_default() must be called; or only enable one ring/aws-lc-rs feature." ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
flake.nix(1 hunks)web-transport-quiche/README.md(1 hunks)web-transport-quiche/src/client.rs(1 hunks)web-transport-quiche/src/connection.rs(1 hunks)web-transport-quiche/src/error.rs(1 hunks)web-transport-quiche/src/ez/client.rs(1 hunks)web-transport-quiche/src/ez/connection.rs(1 hunks)web-transport-quiche/src/ez/driver.rs(1 hunks)web-transport-quiche/src/ez/mod.rs(1 hunks)web-transport-quiche/src/ez/recv.rs(1 hunks)web-transport-quiche/src/ez/send.rs(1 hunks)web-transport-quiche/src/ez/server.rs(1 hunks)web-transport-quiche/src/ez/stream.rs(1 hunks)web-transport-quiche/src/h3/connect.rs(1 hunks)web-transport-quiche/src/h3/mod.rs(1 hunks)web-transport-quiche/src/h3/settings.rs(1 hunks)web-transport-quiche/src/lib.rs(1 hunks)web-transport-quiche/src/recv.rs(1 hunks)web-transport-quiche/src/send.rs(1 hunks)web-transport-quiche/src/server.rs(1 hunks)web-transport-quinn/src/client.rs(1 hunks)web-transport-quinn/src/crypto.rs(2 hunks)web-transport-trait/src/util.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- web-transport-quinn/src/client.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- web-transport-quiche/src/h3/settings.rs
- web-transport-quiche/src/ez/stream.rs
- web-transport-quiche/src/error.rs
- web-transport-quiche/src/ez/mod.rs
- web-transport-quiche/src/server.rs
- web-transport-quiche/README.md
🧰 Additional context used
🧬 Code graph analysis (13)
web-transport-quiche/src/recv.rs (2)
web-transport-quiche/src/ez/recv.rs (12)
std(149-149)std(280-280)new(56-69)new(222-224)read(234-239)read_chunk(244-246)read_buf(277-290)read_all(293-298)close(303-310)closed(342-344)drop(348-361)poll_read(365-376)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-quiche/src/h3/connect.rs (3)
web-transport-quiche/src/h3/request.rs (2)
url(30-32)accept(14-27)web-transport-quiche/src/ez/stream.rs (3)
from(70-72)from(76-78)from(82-84)web-transport-proto/src/connect.rs (3)
from(58-60)read(110-124)read(184-198)
web-transport-quiche/src/ez/client.rs (3)
web-transport-quiche/src/client.rs (7)
default(32-34)with_metrics(41-43)with_socket(50-52)with_bind(57-61)with_settings(67-69)with_cert(72-74)connect(79-86)web-transport-quiche/src/ez/driver.rs (5)
new(39-58)new(149-162)new(548-555)accept_bi(212-241)accept_uni(243-263)web-transport-quiche/src/ez/connection.rs (4)
new(81-83)new(128-143)accept_bi(146-151)accept_uni(154-159)
web-transport-quiche/src/client.rs (2)
web-transport-quiche/src/connection.rs (4)
url(311-313)new(68-107)new(384-406)connect(135-147)web-transport-quiche/src/ez/client.rs (7)
default(20-22)with_metrics(27-37)with_socket(42-60)with_bind(65-69)with_settings(75-78)with_cert(81-88)connect(93-157)
web-transport-quiche/src/ez/server.rs (4)
web-transport-quiche/src/server.rs (3)
default(34-36)new(120-125)accept(130-143)web-transport-quiche/src/ez/driver.rs (5)
new(39-58)new(149-162)new(548-555)accept_bi(212-241)accept_uni(243-263)web-transport-quiche/src/ez/connection.rs (4)
new(81-83)new(128-143)accept_bi(146-151)accept_uni(154-159)web-transport-quiche/src/ez/lock.rs (1)
new(22-26)
web-transport-quiche/src/h3/mod.rs (3)
web-transport-quiche/src/client.rs (1)
connect(79-86)web-transport-quiche/src/connection.rs (1)
connect(135-147)web-transport-quiche/src/h3/settings.rs (1)
connect(40-47)
web-transport-quiche/src/send.rs (2)
web-transport-proto/src/error.rs (1)
error_to_http3(16-18)web-transport-quiche/src/ez/send.rs (13)
new(53-65)new(217-219)write(227-230)write_buf(269-271)write_all(258-264)write_buf_all(274-279)finish(285-305)close(315-322)closed(354-356)drop(372-385)poll_write(389-399)poll_flush(401-404)poll_shutdown(406-417)
web-transport-quiche/src/lib.rs (1)
web-transport-quiche/src/ez/driver.rs (2)
recv(81-88)send(72-79)
web-transport-quiche/src/ez/connection.rs (2)
web-transport-quiche/src/ez/driver.rs (15)
abort(447-452)new(39-58)new(149-162)new(548-555)poll(299-391)is_closed(68-70)close(60-62)wait(295-297)accept_bi(212-241)accept_uni(243-263)closed(64-66)open_bi(91-111)send(72-79)recv(81-88)open_uni(113-133)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)
web-transport-quiche/src/connection.rs (2)
web-transport-quinn/src/session.rs (17)
url(295-297)connect(116-128)run_closed(96-112)accept_uni(131-141)accept_uni(504-506)accept_bi(144-154)accept_bi(508-510)open_uni(157-169)open_uni(516-518)open_bi(172-184)open_bi(512-514)close(248-258)closed(261-263)closed(524-526)raw(282-293)poll_accept_uni(375-414)poll_accept_bi(441-471)web-transport-proto/src/error.rs (1)
error_to_http3(16-18)
web-transport-quiche/src/ez/driver.rs (4)
web-transport-quiche/src/ez/recv.rs (8)
new(56-69)new(222-224)close(303-310)closed(342-344)is_closed(209-211)is_closed(318-320)id(227-229)drop(348-361)web-transport-quiche/src/ez/connection.rs (15)
new(81-83)new(128-143)close(85-91)close(193-196)closed(199-201)is_closed(70-72)is_closed(97-99)is_closed(206-208)open_bi(164-174)open_uni(179-187)accept_bi(146-151)accept_uni(154-159)drop(103-105)wait(93-95)poll(59-68)web-transport-quiche/src/ez/send.rs (8)
new(53-65)new(217-219)close(315-322)closed(354-356)is_closed(204-206)is_closed(330-332)id(222-224)drop(372-385)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)
web-transport-quiche/src/ez/send.rs (3)
web-transport-quiche/src/ez/recv.rs (11)
new(56-69)new(222-224)id(227-229)poll_closed(107-118)poll_closed(322-332)flush(120-207)is_closed(209-211)is_closed(318-320)close(303-310)closed(342-344)drop(348-361)web-transport-quiche/src/ez/driver.rs (7)
new(39-58)new(149-162)new(548-555)is_closed(68-70)write(265-293)close(60-62)closed(64-66)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)
web-transport-quiche/src/ez/recv.rs (3)
web-transport-quiche/src/ez/driver.rs (4)
is_closed(68-70)read(176-210)close(60-62)closed(64-66)web-transport-quiche/src/ez/send.rs (7)
id(222-224)flush(114-192)is_closed(204-206)is_closed(330-332)close(315-322)closed(354-356)drop(372-385)web-transport-quiche/src/recv.rs (8)
read(29-31)read(88-90)close(55-57)close(97-99)closed(60-62)closed(101-103)drop(66-71)poll_read(75-82)
⏰ 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 (14)
flake.nix (1)
33-33: LGTM! Correctly adds C++ standard library for native dependencies.This addition is appropriate for the new web-transport-quiche crate, which requires C++ libraries (e.g., for BoringSSL). Adding it to packages ensures the library is available for the shellHook below.
web-transport-quiche/src/ez/server.rs (1)
25-86: Type‑state builder for listeners/metrics/settings looks solidThe
ServerBuilder<M, S>state parameter cleanly enforces “at least one listener beforewith_cert”, and allwith_*methods correctly preserve theM: Metricstype and propagateSettings. TLS setup viaTlsCertificatePathsandlisten_with_capabilitiesis also wired consistently. No issues here.Also applies to: 88-140
web-transport-trait/src/util.rs (1)
1-32: Conditional MaybeSend/MaybeSync design is correct and consistentThe cfg‑split traits give you
Send/Syncbounds on native and no‑op markers on wasm via blanket impls, matching the documented intent and avoiding portability hacks elsewhere. Looks good.web-transport-quiche/src/lib.rs (1)
1-43: Crate docs and public API wiring look coherentThe top‑level documentation accurately describes the layering, and the module/re‑export layout (
ez,h3, and the top‑level client/connection/server APIs plus TLS/Settings types) is consistent and easy to consume. No issues spotted here.web-transport-quiche/src/h3/mod.rs (1)
1-12: h3 module aggregation is straightforward and correctThe module cleanly groups CONNECT/request/SETTINGS helpers and re‑exports them under
h3::*, matching the documented “handshake helpers” role. No changes needed.web-transport-quiche/src/client.rs (1)
9-75: ClientError and ClientBuilder wrapper are well-structuredThe error type cleanly aggregates IO, SETTINGS, and CONNECT failures, and using
Arc<std::io::Error>makes the enum clonable without losing error detail. TheClientBuilder<M>newtype forwards toez::ClientBuilder<M>while preserving the metrics type and keeping the public API small. This all looks good.web-transport-quiche/src/ez/driver.rs (6)
1-20: LGTM!The imports are well-organized and the DROP_CODE constant is clearly documented with both its ASCII meaning and decimal value.
38-58: LGTM!Stream ID initialization correctly accounts for server vs client roles, following QUIC conventions.
90-133: LGTM!The stream opening logic correctly handles capacity constraints, queues wakers when capacity is exhausted, and safely manages driver wakeups.
299-391: LGTM!The main poll loop correctly coordinates stream creation, capacity updates, and stream flushing. The lock management properly avoids deadlocks by waking tasks after releasing locks.
504-537: LGTM!The connection close handling comprehensively determines the error reason and correctly propagates it to all waiting tasks via both remote and local waker lists.
164-174: LGTM!The remaining driver methods follow consistent patterns for error handling, waker management, and stream lifecycle coordination. The overall architecture is clean and well-integrated with tokio_quiche.
Also applies to: 176-210, 265-293, 393-453, 540-556
web-transport-quinn/src/crypto.rs (2)
1-13: LGTM!The module documentation is clear, imports are appropriate, and the
Providertype alias provides a clean ergonomic wrapper for shared crypto providers.
51-65: The original review concern is based on an incorrect premise.Accessing hash providers via cipher suites is the only official API provided by rustls's CryptoProvider structure. There is no more direct method. Standard providers (ring and aws-lc-rs, with aws-lc-rs being the default) both expose TLS 1.3 cipher suites with SHA-256, making this approach robust for all typical configurations. The code correctly handles edge cases by using
tls13()?to filter TLS 1.3 suites and panics appropriately if no SHA-256 provider is found, which is the correct error handling for invalid provider configurations.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
web-transport-quiche/src/ez/send.rs (2)
19-21: Critical: DROP_CODE still exceeds QUIC varint limit.Despite the previous review comment being marked as addressed,
0x656E646464726F70(7308889627613622128) still exceeds the HTTP/3/QUIC varint ceiling of2^62. Calls tostream_shutdownat line 118 will reject this code.Apply the fix:
-// "senddrop" in ascii; if you see this then call finish().await or close(code) -// decimal: 7308889627613622128 -const DROP_CODE: u64 = 0x656E646464726F70; +// Derive a valid HTTP/3 error code for dropped send streams +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x73656E64); // "send" prefix
152-152: Critical: Capacity underflow still present.Despite being flagged previously, line 152 still performs
self.capacity -= nwhich can underflow whencapacityis already 0 (after being debited inpoll_write_bufat line 92). This causes a panic in debug builds or wraps to a bogus huge value in release builds until the reassignment at line 175.Apply the previously suggested fix:
- self.capacity -= n; + self.capacity = self.capacity.saturating_sub(n);web-transport-quiche/src/ez/recv.rs (1)
20-22: Critical: DROP_CODE exceeds QUIC varint limit.Despite the previous review comment,
0x6563766464726F70(7305813194079104880) still exceeds the HTTP/3/QUIC varint ceiling of2^62. The STOP_SENDING frame sent at line 128 will be rejected.Apply the fix:
-// "recvdrop" in ascii; if you see this then read everything or close(code) -// decimal: 7305813194079104880 -const DROP_CODE: u64 = 0x6563766464726F70; +// Derive a valid HTTP/3 error code for dropped recv streams +const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x72656376); // "recv" prefix
🧹 Nitpick comments (2)
web-transport-quiche/src/client.rs (1)
79-93: Consider validating the URL scheme.The method validates the host but not the scheme. WebTransport typically expects
https://URLs. Adding a scheme check could provide clearer error messages for users passing invalid URL schemes.For example:
pub async fn connect(self, url: Url) -> Result<Connection, ClientError> { if url.scheme() != "https" { return Err(ClientError::InvalidUrl(format!( "expected https:// scheme, got: {}://", url.scheme() ))); } let port = url.port().unwrap_or(443); let host = match url.host() { Some(host) => host.to_string(), None => return Err(ClientError::InvalidUrl(url.to_string())), }; let conn = self.0.connect(&host, port).await?; Connection::connect(conn, url).await }web-transport-quiche/src/ez/driver.rs (1)
467-470: Complete theshould_act()implementation.The method always returns
truewith a TODO comment. This may cause unnecessary polling cycles and waste CPU resources. Consider implementing proper logic to determine when the driver should act based on pending state (e.g., check if there are streams to flush, data to send, or capacity changes).Would you like me to generate an implementation that checks for pending work?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
flake.nix(1 hunks)web-transport-proto/Cargo.toml(1 hunks)web-transport-quiche/src/client.rs(1 hunks)web-transport-quiche/src/ez/driver.rs(1 hunks)web-transport-quiche/src/ez/recv.rs(1 hunks)web-transport-quiche/src/ez/send.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- flake.nix
🧰 Additional context used
🧬 Code graph analysis (4)
web-transport-quiche/src/ez/send.rs (4)
web-transport-quiche/src/ez/recv.rs (13)
std(150-150)std(281-281)new(56-69)new(223-225)id(228-230)poll_closed(107-118)poll_closed(323-333)flush(121-208)is_closed(210-212)is_closed(319-321)close(304-311)closed(343-345)drop(349-362)web-transport-quiche/src/ez/driver.rs (7)
new(39-58)new(151-164)new(547-554)is_closed(68-70)write(264-292)close(60-62)closed(64-66)web-transport-quiche/src/send.rs (11)
new(26-28)write(31-33)write(116-118)write_all(41-43)finish(54-56)finish(128-130)close(68-71)close(124-126)closed(74-76)closed(132-134)drop(80-86)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)
web-transport-quiche/src/client.rs (2)
web-transport-quiche/src/connection.rs (4)
url(311-313)new(68-107)new(384-406)connect(135-147)web-transport-quiche/src/ez/client.rs (7)
default(20-22)with_metrics(27-37)with_socket(42-60)with_bind(65-69)with_settings(75-78)with_cert(81-88)connect(93-157)
web-transport-quiche/src/ez/driver.rs (3)
web-transport-quiche/src/ez/recv.rs (9)
new(56-69)new(223-225)close(304-311)closed(343-345)is_closed(210-212)is_closed(319-321)id(228-230)read(235-240)drop(349-362)web-transport-quiche/src/ez/send.rs (9)
new(53-65)new(218-220)close(316-323)closed(355-357)is_closed(205-207)is_closed(331-333)id(223-225)drop(373-386)write(228-231)web-transport-quiche/src/ez/connection.rs (15)
new(81-83)new(128-143)close(85-91)close(193-196)closed(199-201)is_closed(70-72)is_closed(97-99)is_closed(206-208)open_bi(164-174)open_uni(179-187)accept_bi(146-151)accept_uni(154-159)drop(103-105)wait(93-95)poll(59-68)
web-transport-quiche/src/ez/recv.rs (3)
web-transport-quiche/src/ez/driver.rs (7)
new(39-58)new(151-164)new(547-554)is_closed(68-70)read(178-212)close(60-62)closed(64-66)web-transport-quiche/src/ez/send.rs (9)
new(53-65)new(218-220)id(223-225)flush(115-193)is_closed(205-207)is_closed(331-333)close(316-323)closed(355-357)drop(373-386)web-transport-quiche/src/ez/lock.rs (2)
new(22-26)drop(41-43)
⏰ 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 (4)
web-transport-proto/Cargo.toml (1)
20-21: ✅ Critical issue resolved correctly.The tokio dependency now correctly includes the
io-utilfeature while disabling defaults, ensuringAsyncReadandAsyncWritetraits remain available for async IO helpers throughout the PR.web-transport-quiche/src/client.rs (3)
1-29: LGTM! Well-structured error types.The error hierarchy is clean, and wrapping
std::io::ErrorinArcto makeClientErrorcloneable is a good pattern. The automatic conversions via#[from]for H3 errors keep call sites concise.
31-47: LGTM! Clean builder API design.The newtype wrapper around
ez::ClientBuilderprovides a clean separation of concerns, and the metrics-aware API is well-structured.
85-88: Great fix! Missing host now returns a proper error.This resolves the past review comment about panicking on URLs without a host. The match expression now gracefully returns
ClientError::InvalidUrlinstead of panicking.
| /// **WARNING**: [Settings::verify_peer] is set to false by default. | ||
| /// This will completely bypass certificate verification and is generally not recommended. |
There was a problem hiding this comment.
Incorrect security warning in documentation.
The docstring warns that verify_peer is set to false by default, but the code in ez::ClientBuilder::with_metrics (ez/client.rs:29) explicitly sets settings.verify_peer = true. This documentation error is misleading about the security posture of the client.
Apply this diff to correct the warning:
- /// **WARNING**: [Settings::verify_peer] is set to false by default.
- /// This will completely bypass certificate verification and is generally not recommended.
+ /// [Settings::verify_peer] is set to true by default to ensure secure connections.
+ /// Set it to false only in testing environments where certificate verification should be bypassed.Based on learnings
🤖 Prompt for AI Agents
In web-transport-quiche/src/client.rs around lines 68 to 69, the docstring
incorrectly states that Settings::verify_peer is false by default; update the
comment to reflect that the client builder (ez::ClientBuilder::with_metrics)
sets settings.verify_peer = true by default. Replace the warning text with a
corrected note that verify_peer defaults to true (or that the builder enables
peer verification) and clarify that disabling verification is unsafe if changed
elsewhere, ensuring the docstring matches the actual code behavior.
| // "conndrop" in ascii; if you see this then close(code) | ||
| // decimal: 8029476563109179248 | ||
| const DROP_CODE: u64 = 0x6F6E6E6464726F70; |
There was a problem hiding this comment.
Critical: DROP_CODE exceeds QUIC varint limit.
The constant 0x6F6E6E6464726F70 equals 8029476563109179248, which exceeds the QUIC varint ceiling of 2^62 (4611686018427387904). When qconn.close() attempts to send this code at line 312, it will fail or be rejected by the peer.
Use web_transport_proto::error_to_http3(...) to derive a valid HTTP/3 error code:
-// "conndrop" in ascii; if you see this then close(code)
-// decimal: 8029476563109179248
-const DROP_CODE: u64 = 0x6F6E6E6464726F70;
+// Derive a valid HTTP/3 error code for dropped connections
+const DROP_CODE: u64 = web_transport_proto::error_to_http3(0x636F6E6E); // "conn" prefixCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web-transport-quiche/src/ez/driver.rs around lines 18 to 20, the hardcoded
DROP_CODE (0x6F6E6E6464726F70 / 8029476563109179248) exceeds the QUIC varint max
(2^62) and must not be sent; remove the oversized constant and instead derive a
legal HTTP/3 error code by calling web_transport_proto::error_to_http3(...) when
closing qconn (or map your internal error to an HTTP/3 error via that helper) so
the value sent is within the QUIC varint limit; replace usages of DROP_CODE with
the value returned by error_to_http3(...) (or a checked/converted version)
before calling qconn.close().
This reverts commit 031c93f.
Summary by CodeRabbit
New Features
Documentation
Refactor