Skip to content

Commit 8164687

Browse files
englishmenglishm
authored
Merge pull request #115 from cloudflare/me/better-document-unimplemented-features
Improve error handling
2 parents 4ae7944 + b22e5dd commit 8164687

File tree

5 files changed

+78
-38
lines changed

5 files changed

+78
-38
lines changed

‎moq-transport/src/serve/error.rs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,33 @@ pub enum ServeError {
2424

2525
#[error("internal error: {0}")]
2626
Internal(String),
27+
28+
#[error("not implemented: {0}")]
29+
NotImplemented(String),
2730
}
2831

2932
impl ServeError {
33+
/// Returns error codes for per-request/per-track errors.
34+
/// These codes are used in SUBSCRIBE_ERROR, PUBLISH_DONE, FETCH_ERROR, etc.
35+
/// Error codes are based on draft-ietf-moq-transport-14 Section 13.1.x
3036
pub fn code(&self) -> u64 {
3137
match self {
38+
// Special case: 0 typically means successful completion or internal error depending on context
3239
Self::Done => 0,
40+
// Cancel/Going away - maps to various contexts
3341
Self::Cancel => 1,
42+
// Pass through application-specific error codes
3443
Self::Closed(code) => *code,
35-
Self::NotFound => 404,
36-
Self::Duplicate => 409,
37-
Self::Mode => 400,
38-
Self::Size => 413,
39-
Self::Internal(_) => 500,
44+
// TRACK_DOES_NOT_EXIST (0x4) from SUBSCRIBE_ERROR codes
45+
Self::NotFound => 0x4,
46+
// This is more of a session-level error, but keeping a reasonable code
47+
Self::Duplicate => 0x5,
48+
// NOT_SUPPORTED (0x3) - appears in multiple error code registries
49+
Self::Mode => 0x3,
50+
Self::Size => 0x3,
51+
Self::NotImplemented(_) => 0x3,
52+
// INTERNAL_ERROR (0x0) - per-request error registries use 0x0
53+
Self::Internal(_) => 0x0,
4054
}
4155
}
4256
}

‎moq-transport/src/session/error.rs‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,39 @@ pub enum SessionError {
4444
WrongSize,
4545
}
4646

47-
// TODO SLG - update with errors from moq-transport draft-13 section 3.4 Termination
47+
// Session Termination Error Codes from draft-ietf-moq-transport-14 Section 13.1.1
4848
impl SessionError {
4949
/// An integer code that is sent over the wire.
50+
/// Returns Session Termination Error Codes per draft-14.
5051
pub fn code(&self) -> u64 {
5152
match self {
52-
Self::RoleViolation => 405,
53-
Self::Session(_) => 503,
54-
Self::Read(_) => 500,
55-
Self::Write(_) => 500,
56-
Self::Version(..) => 406,
57-
Self::Decode(_) => 400,
58-
Self::Encode(_) => 500,
59-
Self::BoundsExceeded(_) => 500,
60-
Self::Duplicate => 409,
61-
Self::Internal => 500,
62-
Self::WrongSize => 400,
53+
// PROTOCOL_VIOLATION (0x3) - The role negotiated in the handshake was violated
54+
Self::RoleViolation => 0x3,
55+
// INTERNAL_ERROR (0x1) - Generic internal errors
56+
Self::Session(_) => 0x1,
57+
Self::Read(_) => 0x1,
58+
Self::Write(_) => 0x1,
59+
Self::Encode(_) => 0x1,
60+
Self::BoundsExceeded(_) => 0x1,
61+
Self::Internal => 0x1,
62+
// VERSION_NEGOTIATION_FAILED (0x15)
63+
Self::Version(..) => 0x15,
64+
// PROTOCOL_VIOLATION (0x3) - Malformed messages
65+
Self::Decode(_) => 0x3,
66+
Self::WrongSize => 0x3,
67+
// DUPLICATE_TRACK_ALIAS (0x5)
68+
Self::Duplicate => 0x5,
69+
// Delegate to ServeError for per-request error codes
6370
Self::Serve(err) => err.code(),
6471
}
6572
}
73+
74+
/// Helper for unimplemented protocol features
75+
/// Logs a warning and returns a NotImplemented error instead of panicking
76+
pub fn unimplemented(feature: &str) -> Self {
77+
log::warn!("Protocol feature not implemented: {}", feature);
78+
Self::Serve(serve::ServeError::NotImplemented(feature.to_string()))
79+
}
6680
}
6781

6882
impl From<SessionError> for serve::ServeError {

‎moq-transport/src/session/mod.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,11 @@ impl Session {
341341
};
342342

343343
// TODO GOAWAY, MAX_REQUEST_ID, REQUESTS_BLOCKED
344-
unimplemented!("unknown message context: {:?}", msg)
344+
log::warn!("Unimplemented message type received: {:?}", msg);
345+
return Err(SessionError::unimplemented(&format!(
346+
"message type {:?}",
347+
msg
348+
)));
345349
}
346350
}
347351

‎moq-transport/src/session/publisher.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,20 +210,28 @@ impl Publisher {
210210
message::Subscriber::Subscribe(msg) => self.recv_subscribe(msg),
211211
message::Subscriber::SubscribeUpdate(msg) => self.recv_subscribe_update(msg),
212212
message::Subscriber::Unsubscribe(msg) => self.recv_unsubscribe(msg),
213-
message::Subscriber::Fetch(_msg) => todo!(), // TODO
214-
message::Subscriber::FetchCancel(_msg) => todo!(), // TODO
213+
message::Subscriber::Fetch(_msg) => Err(SessionError::unimplemented("FETCH")),
214+
message::Subscriber::FetchCancel(_msg) => {
215+
Err(SessionError::unimplemented("FETCH_CANCEL"))
216+
}
215217
message::Subscriber::TrackStatus(msg) => self.recv_track_status(msg),
216-
message::Subscriber::SubscribeNamespace(_msg) => todo!(), // TODO
217-
message::Subscriber::UnsubscribeNamespace(_msg) => todo!(), // TODO
218+
message::Subscriber::SubscribeNamespace(_msg) => {
219+
Err(SessionError::unimplemented("SUBSCRIBE_NAMESPACE"))
220+
}
221+
message::Subscriber::UnsubscribeNamespace(_msg) => {
222+
Err(SessionError::unimplemented("UNSUBSCRIBE_NAMESPACE"))
223+
}
218224
message::Subscriber::PublishNamespaceCancel(msg) => {
219225
self.recv_publish_namespace_cancel(msg)
220226
}
221227
message::Subscriber::PublishNamespaceOk(msg) => self.recv_publish_namespace_ok(msg),
222228
message::Subscriber::PublishNamespaceError(msg) => {
223229
self.recv_publish_namespace_error(msg)
224230
}
225-
message::Subscriber::PublishOk(_msg) => todo!(), // TODO
226-
message::Subscriber::PublishError(_msg) => todo!(), // TODO
231+
message::Subscriber::PublishOk(_msg) => Err(SessionError::unimplemented("PUBLISH_OK")),
232+
message::Subscriber::PublishError(_msg) => {
233+
Err(SessionError::unimplemented("PUBLISH_ERROR"))
234+
}
227235
};
228236

229237
if let Err(err) = res {
@@ -329,7 +337,7 @@ impl Publisher {
329337
_msg: message::SubscribeUpdate,
330338
) -> Result<(), SessionError> {
331339
// TODO: Implement updating subscriptions.
332-
Err(SessionError::Internal)
340+
Err(SessionError::unimplemented("SUBSCRIBE_UPDATE"))
333341
}
334342

335343
fn recv_track_status(&mut self, msg: message::TrackStatus) -> Result<(), SessionError> {

‎moq-transport/src/session/subscriber.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -120,35 +120,35 @@ impl Subscriber {
120120
self.drop_publish_namespace(&msg.track_namespace)
121121
}
122122
// TODO SLG - there is no longer a namespace in the error, need to map via request id
123-
message::Subscriber::PublishNamespaceError(_msg) => todo!(), //self.drop_announce(&msg.track_namespace),
123+
message::Subscriber::PublishNamespaceError(_msg) => {} // Not implemented yet - need request id mapping
124124
_ => {}
125125
}
126126

127127
// TODO report dropped messages?
128128
let _ = self.outgoing.push(msg.into());
129129
}
130130

131-
fn not_implemented_yet(&self) -> Result<(), SessionError> {
132-
Err(SessionError::Serve(ServeError::Internal(
133-
"Not implemented yet".to_string(),
134-
)))
135-
}
136-
137131
/// Receive a message from the publisher via the control stream.
138132
pub(super) fn recv_message(&mut self, msg: message::Publisher) -> Result<(), SessionError> {
139133
let res = match &msg {
140134
message::Publisher::PublishNamespace(msg) => self.recv_publish_namespace(msg),
141135
message::Publisher::PublishNamespaceDone(msg) => self.recv_publish_namespace_done(msg),
142-
message::Publisher::Publish(_msg) => self.not_implemented_yet(), // TODO
136+
message::Publisher::Publish(_msg) => Err(SessionError::unimplemented("PUBLISH")),
143137
message::Publisher::PublishDone(msg) => self.recv_publish_done(msg),
144138
message::Publisher::SubscribeOk(msg) => self.recv_subscribe_ok(msg),
145139
message::Publisher::SubscribeError(msg) => self.recv_subscribe_error(msg),
146140
message::Publisher::TrackStatusOk(msg) => self.recv_track_status_ok(msg),
147-
message::Publisher::TrackStatusError(_msg) => self.not_implemented_yet(), // TODO
148-
message::Publisher::FetchOk(_msg) => self.not_implemented_yet(), // TODO
149-
message::Publisher::FetchError(_msg) => self.not_implemented_yet(), // TODO
150-
message::Publisher::SubscribeNamespaceOk(_msg) => self.not_implemented_yet(), // TODO
151-
message::Publisher::SubscribeNamespaceError(_msg) => self.not_implemented_yet(), // TODO
141+
message::Publisher::TrackStatusError(_msg) => {
142+
Err(SessionError::unimplemented("TRACK_STATUS_ERROR"))
143+
}
144+
message::Publisher::FetchOk(_msg) => Err(SessionError::unimplemented("FETCH_OK")),
145+
message::Publisher::FetchError(_msg) => Err(SessionError::unimplemented("FETCH_ERROR")),
146+
message::Publisher::SubscribeNamespaceOk(_msg) => {
147+
Err(SessionError::unimplemented("SUBSCRIBE_NAMESPACE_OK"))
148+
}
149+
message::Publisher::SubscribeNamespaceError(_msg) => {
150+
Err(SessionError::unimplemented("SUBSCRIBE_NAMESPACE_ERROR"))
151+
}
152152
};
153153

154154
if let Err(SessionError::Serve(err)) = res {

0 commit comments

Comments
 (0)