Make SUBSCRIBE_NAMESPACE errors non-fatal

[kixelated]

Summary

• When the peer rejects our SUBSCRIBE_NAMESPACE request, log a warning and continue the session instead of tearing it down
• The session can still function via unsolicited PUBLISH_NAMESPACE messages from the peer
• Fixes the issue where clients responding with DOES_NOT_EXIST or UNINTERESTED to SUBSCRIBE_NAMESPACE would cause the relay to drop the session

Test plan

• Connect a draft-17 client that rejects SUBSCRIBE_NAMESPACE — session should stay alive
• Verify that PUBLISH_NAMESPACE announcements still flow after SUBSCRIBE_NAMESPACE is rejected

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

The error handling for run_subscribe_namespace calls in the session module is modified to catch errors and log them at warn level instead of propagating them directly. When an error occurs, the code logs the error and returns Ok(()), preventing subscribe_namespace failures from terminating the surrounding tokio::select! block. This change applies uniformly across both IETF Draft14–16 and non-Draft code paths in the session handling logic.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title directly and concisely summarizes the main change: making SUBSCRIBE_NAMESPACE errors non-fatal instead of failing the session.
Description check	✅ Passed	The description is clearly related to the changeset, explaining the rationale for the error handling change and providing context about PUBLISH_NAMESPACE fallback behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch publish_namespace_feedback

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch publish_namespace_feedback

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rs/moq-lite/src/ietf/session.rs`:
- Around line 55-58: The current handling of sub_ns.run_subscribe_namespace(...)
swallows all errors; change both call sites so that only the expected rejection
variant (match Err(Error::Cancel { .. })) is downgraded to tracing::warn!(..);
return Err(err) (or propagate the error) for any other error variants (e.g.,
Transport/Decode/ProtocolViolation) instead of Ok(()). Locate the two places
where run_subscribe_namespace is awaited and replace the unconditional if let
Err(err) => { warn; Ok(()) } with a match on the error that warns and continues
only for Error::Cancel and otherwise propagates the error up to the caller
(preserving the original Err type).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41d5ee34-a304-4596-a111-b5db7a2f5321

📥 Commits

Reviewing files that changed from the base of the PR and between 69d8ff9 and 6dcd665.

📒 Files selected for processing (1)

• rs/moq-lite/src/ietf/session.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Only downgrade expected SUBSCRIBE_NAMESPACE rejections; propagate real failures.

On Line 55 and Line 89, all run_subscribe_namespace errors are converted to warn+Ok(()). That also suppresses Transport, Decode, ProtocolViolation, etc., which can leave the session in an invalid state. This should continue only for the expected rejection path (currently Error::Cancel) and return other errors to tokio::select!.

Suggested fix

-						if let Err(err) = sub_ns.run_subscribe_namespace(stream).await {
-						tracing::warn!(%err, "subscribe_namespace failed, continuing without");
-					}
-					Ok(())
+						match sub_ns.run_subscribe_namespace(stream).await {
+							Ok(()) => Ok(()),
+							Err(Error::Cancel) => {
+								tracing::warn!("subscribe_namespace rejected, continuing without");
+								Ok(())
+							}
+							Err(err) => Err(err),
+						}
 					} => Err(err),

-						if let Err(err) = sub_ns.run_subscribe_namespace(stream).await {
-							tracing::warn!(%err, "subscribe_namespace failed, continuing without");
-						}
-						Ok(())
+						match sub_ns.run_subscribe_namespace(stream).await {
+							Ok(()) => Ok(()),
+							Err(Error::Cancel) => {
+								tracing::warn!("subscribe_namespace rejected, continuing without");
+								Ok(())
+							}
+							Err(err) => Err(err),
+						}
 					} => Err(err),

Also applies to: 89-92

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@rs/moq-lite/src/ietf/session.rs` around lines 55 - 58, The current handling
of sub_ns.run_subscribe_namespace(...) swallows all errors; change both call
sites so that only the expected rejection variant (match Err(Error::Cancel { ..
})) is downgraded to tracing::warn!(..); return Err(err) (or propagate the
error) for any other error variants (e.g., Transport/Decode/ProtocolViolation)
instead of Ok(()). Locate the two places where run_subscribe_namespace is
awaited and replace the unconditional if let Err(err) => { warn; Ok(()) } with a
match on the error that warns and continues only for Error::Cancel and otherwise
propagates the error up to the caller (preserving the original Err type).