Conversation
H3 requires WebTransport framing which not all H3 endpoints support, so advertise it last. Also remove the +iroh scheme variants (moql+iroh, moqt+iroh, etc.) in favor of just "iroh". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughWorkspace manifest updated: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # Cargo.lock # rs/moq-native/Cargo.toml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@rs/moq-native/src/iroh.rs`:
- Around line 72-82: Add unit tests covering the bind() method, the connect()
function, and the url_set_scheme() utility: write tests that validate ALPN
ordering remains moq-lite entries before web_transport_iroh::ALPN_H3, that
bind() correctly binds or returns errors for v4/v6 addresses, that connect()
establishes (or errors) under expected conditions, and that url_set_scheme()
updates URL schemes correctly for typical inputs; place tests in the module's
tests section or a #[cfg(test)] mod with targeted cases exercising normal, edge,
and error paths and referencing the functions bind(), connect(), and
url_set_scheme() so CI verifies behavior.
🧹 Nitpick comments (1)
rs/moq-native/src/iroh.rs (1)
159-167: Inconsistent H3 ALPN: hardcodedb"h3"vs.web_transport_iroh::ALPN_H3.Line 74 and line 174 use the
web_transport_iroh::ALPN_H3constant, but line 166 hardcodesb"h3". Use the constant for consistency to avoid a silent mismatch if the value ever changes.Suggested fix
- additional.push(b"h3".to_vec()); + additional.push(web_transport_iroh::ALPN_H3.as_bytes().to_vec());
| // H3 is last because it requires WebTransport framing which not all H3 endpoints support. | ||
| let mut alpns: Vec<Vec<u8>> = moq_lite::ALPNS.iter().map(|alpn| alpn.as_bytes().to_vec()).collect(); | ||
| alpns.push(web_transport_iroh::ALPN_H3.as_bytes().to_vec()); | ||
|
|
||
| let mut builder = IrohEndpoint::builder().secret_key(secret_key).alpns(alpns); | ||
| if let Some(addr) = self.bind_v4 { | ||
| builder = builder.bind_addr_v4(addr); | ||
| builder = builder.bind_addr(addr)?; | ||
| } | ||
| if let Some(addr) = self.bind_v6 { | ||
| builder = builder.bind_addr_v6(addr); | ||
| builder = builder.bind_addr(addr)?; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if bind_addr is used elsewhere and what type it returns
rg -n 'bind_addr' --type=rust -C3Repository: moq-dev/moq
Length of output: 627
🏁 Script executed:
#!/bin/bash
# Get more context around the lines in question and check the full file
head -100 rs/moq-native/src/iroh.rs | tail -40Repository: moq-dev/moq
Length of output: 1132
🏁 Script executed:
#!/bin/bash
# Check for other backends to compare ALPN patterns
rg -n 'ALPN' --type=rust -C2 | head -60Repository: moq-dev/moq
Length of output: 3931
🏁 Script executed:
#!/bin/bash
# Check for tests in the iroh.rs file
rg -n '#\[test\]|#\[cfg\(test\)\]|mod tests' rs/moq-native/src/iroh.rsRepository: moq-dev/moq
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Get the full file to understand its structure
wc -l rs/moq-native/src/iroh.rsRepository: moq-dev/moq
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Read the full file to assess structure and test integration
cat -n rs/moq-native/src/iroh.rsRepository: moq-dev/moq
Length of output: 8442
ALPN ordering and binding are correct, but tests are missing per Rust file guidelines.
The ALPN priority (moq-lite first, H3 last) is correct and consistently applied throughout the file. The builder.bind_addr(addr)? calls correctly handle the fallible API by propagating errors.
However, this Rust file lacks integrated tests. The bind() method, connect() function, and url_set_scheme() utility should have unit tests.
🤖 Prompt for AI Agents
In `@rs/moq-native/src/iroh.rs` around lines 72 - 82, Add unit tests covering the
bind() method, the connect() function, and the url_set_scheme() utility: write
tests that validate ALPN ordering remains moq-lite entries before
web_transport_iroh::ALPN_H3, that bind() correctly binds or returns errors for
v4/v6 addresses, that connect() establishes (or errors) under expected
conditions, and that url_set_scheme() updates URL schemes correctly for typical
inputs; place tests in the module's tests section or a #[cfg(test)] mod with
targeted cases exercising normal, edge, and error paths and referencing the
functions bind(), connect(), and url_set_scheme() so CI verifies behavior.
1 participant
Summary
moql+iroh,moqt+iroh,moqt-15+iroh,h3+irohURL schemes — onlyirohis supported nowTest plan
iroh://URLs connect successfully🤖 Generated with Claude Code