Skip to content

Commit 3c87e69

Browse files
metrics: disambiguate websocket receive disconnects (#5547)
# Description of Changes Adds specific `ClientDisconnectCause` variants for each websocket receive `WsError` variant, and records those specific causes in the subscribe route instead of collapsing all receive failures into `websocket_receive_error`. The legacy `WebsocketReceiveError` enum variant and metric label are retained, but the receive loop now emits the more specific labels. # API and ABI breaking changes This adds public Rust enum variants and new metric label values. Existing variants are retained. # Expected complexity level and risk 1 This is an observability-only change. Websocket control flow is unchanged. # Testing - [x] `cargo fmt --all -- --check` - [x] `cargo test -p spacetimedb-core client_disconnect_cause_labels_are_unique` - [x] `cargo test -p spacetimedb-client-api recv_loop_terminates_when_input_yields_err` Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com> Co-authored-by: Tyler Cloutier <cloutiertyler@users.noreply.github.com>
1 parent 7acce2c commit 3c87e69

2 files changed

Lines changed: 92 additions & 25 deletions

File tree

crates/client-api/src/routes/subscribe.rs

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,23 @@ fn ws_recv_loop(
892892
idle_tx: watch::Sender<Instant>,
893893
mut ws: impl Stream<Item = Result<WsMessage, WsError>> + Unpin,
894894
) -> impl Stream<Item = ClientMessage> {
895+
fn receive_error_cause(error: &WsError) -> ClientDisconnectCause {
896+
match error {
897+
WsError::ConnectionClosed => ClientDisconnectCause::WebsocketReceiveConnectionClosed,
898+
WsError::AlreadyClosed => ClientDisconnectCause::WebsocketReceiveAlreadyClosed,
899+
WsError::Io(_) => ClientDisconnectCause::WebsocketReceiveIo,
900+
WsError::Tls(_) => ClientDisconnectCause::WebsocketReceiveTls,
901+
WsError::Capacity(_) => ClientDisconnectCause::WebsocketReceiveCapacity,
902+
WsError::Protocol(_) => ClientDisconnectCause::WebsocketReceiveProtocol,
903+
WsError::WriteBufferFull(_) => ClientDisconnectCause::WebsocketReceiveWriteBufferFull,
904+
WsError::Utf8(_) => ClientDisconnectCause::WebsocketReceiveUtf8,
905+
WsError::AttackAttempt => ClientDisconnectCause::WebsocketReceiveAttackAttempt,
906+
WsError::Url(_) => ClientDisconnectCause::WebsocketReceiveUrl,
907+
WsError::Http(_) => ClientDisconnectCause::WebsocketReceiveHttp,
908+
WsError::HttpFormat(_) => ClientDisconnectCause::WebsocketReceiveHttpFormat,
909+
}
910+
}
911+
895912
// Get the next message from `ws`, or `None` if the stream is exhausted.
896913
//
897914
// If `state.closed`, `ws` is drained until it either yields an `Err`, is
@@ -948,27 +965,13 @@ fn ws_recv_loop(
948965
log::trace!("message received while already closed");
949966
}
950967
// None of the error cases can be meaningfully recovered from
951-
// (and some can't even occur on the `ws` stream).
952-
// Exit here but spell out an exhaustive match
953-
// in order to bring any future library changes to our attention.
954-
Err(e) => match e {
955-
e @ (WsError::ConnectionClosed
956-
| WsError::AlreadyClosed
957-
| WsError::Io(_)
958-
| WsError::Tls(_)
959-
| WsError::Capacity(_)
960-
| WsError::Protocol(_)
961-
| WsError::WriteBufferFull(_)
962-
| WsError::Utf8(_)
963-
| WsError::AttackAttempt
964-
| WsError::Url(_)
965-
| WsError::Http(_)
966-
| WsError::HttpFormat(_)) => {
967-
state.record_disconnect(ClientDisconnectCause::WebsocketReceiveError);
968-
log::warn!("Websocket receive error: {e}");
969-
break;
970-
}
971-
},
968+
// (and some can't even occur on the `ws` stream), so record the
969+
// specific receive error cause and terminate the stream.
970+
Err(e) => {
971+
state.record_disconnect(receive_error_cause(&e));
972+
log::warn!("Websocket receive error: {e}");
973+
break;
974+
}
972975
}
973976
}
974977
}
@@ -2129,7 +2132,8 @@ mod tests {
21292132
#[tokio::test]
21302133
async fn recv_loop_terminates_when_input_yields_err() {
21312134
let state = Arc::new(actor_state_with_disconnect_recorder(2, <_>::default()));
2132-
let before = disconnect_count(state.database, ClientDisconnectCause::WebsocketReceiveError);
2135+
let cause = ClientDisconnectCause::WebsocketReceiveConnectionClosed;
2136+
let before = disconnect_count(state.database, cause);
21332137
let (idle_tx, _idle_rx) = watch::channel(Instant::now() + state.config.idle_timeout);
21342138

21352139
let input = stream::iter(vec![
@@ -2144,7 +2148,7 @@ mod tests {
21442148

21452149
assert_matches!(recv_loop.next().await, Some(ClientMessage::Ping(_)));
21462150
assert_matches!(recv_loop.next().await, None);
2147-
assert_disconnect_count_incremented(state.database, ClientDisconnectCause::WebsocketReceiveError, before);
2151+
assert_disconnect_count_incremented(state.database, cause, before);
21482152
}
21492153

21502154
#[tokio::test]

crates/core/src/worker_metrics/mod.rs

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@ pub enum ClientDisconnectCause {
3939
ClientMessageError,
4040
/// The websocket receive stream returned an error.
4141
WebsocketReceiveError,
42+
/// The websocket receive stream reported that the connection was already closed.
43+
WebsocketReceiveConnectionClosed,
44+
/// The websocket receive stream was polled after it had already closed.
45+
WebsocketReceiveAlreadyClosed,
46+
/// The websocket receive stream returned an IO error.
47+
WebsocketReceiveIo,
48+
/// The websocket receive stream returned a TLS error.
49+
WebsocketReceiveTls,
50+
/// The websocket receive stream returned a capacity error.
51+
WebsocketReceiveCapacity,
52+
/// The websocket receive stream returned a protocol error.
53+
WebsocketReceiveProtocol,
54+
/// The websocket receive stream reported a full write buffer while receiving.
55+
WebsocketReceiveWriteBufferFull,
56+
/// The websocket receive stream returned a UTF-8 error.
57+
WebsocketReceiveUtf8,
58+
/// The websocket receive stream detected an attack attempt.
59+
WebsocketReceiveAttackAttempt,
60+
/// The websocket receive stream returned a URL error.
61+
WebsocketReceiveUrl,
62+
/// The websocket receive stream returned an HTTP error.
63+
WebsocketReceiveHttp,
64+
/// The websocket receive stream returned an HTTP format error.
65+
WebsocketReceiveHttpFormat,
4266
/// The server failed while sending or flushing websocket data.
4367
WebsocketSendError,
4468
/// The websocket receive stream ended without a more specific cause.
@@ -48,14 +72,26 @@ pub enum ClientDisconnectCause {
4872
}
4973

5074
impl ClientDisconnectCause {
51-
pub const ALL: [Self; 10] = [
75+
pub const ALL: [Self; 22] = [
5276
Self::ClientClose,
5377
Self::IdleTimeout,
5478
Self::IncomingQueueFull,
5579
Self::OutgoingQueueFull,
5680
Self::ModuleExited,
5781
Self::ClientMessageError,
5882
Self::WebsocketReceiveError,
83+
Self::WebsocketReceiveConnectionClosed,
84+
Self::WebsocketReceiveAlreadyClosed,
85+
Self::WebsocketReceiveIo,
86+
Self::WebsocketReceiveTls,
87+
Self::WebsocketReceiveCapacity,
88+
Self::WebsocketReceiveProtocol,
89+
Self::WebsocketReceiveWriteBufferFull,
90+
Self::WebsocketReceiveUtf8,
91+
Self::WebsocketReceiveAttackAttempt,
92+
Self::WebsocketReceiveUrl,
93+
Self::WebsocketReceiveHttp,
94+
Self::WebsocketReceiveHttpFormat,
5995
Self::WebsocketSendError,
6096
Self::WebsocketStreamEnded,
6197
Self::Unknown,
@@ -70,6 +106,18 @@ impl ClientDisconnectCause {
70106
Self::ModuleExited => "module_exited",
71107
Self::ClientMessageError => "client_message_error",
72108
Self::WebsocketReceiveError => "websocket_receive_error",
109+
Self::WebsocketReceiveConnectionClosed => "websocket_receive_connection_closed",
110+
Self::WebsocketReceiveAlreadyClosed => "websocket_receive_already_closed",
111+
Self::WebsocketReceiveIo => "websocket_receive_io",
112+
Self::WebsocketReceiveTls => "websocket_receive_tls",
113+
Self::WebsocketReceiveCapacity => "websocket_receive_capacity",
114+
Self::WebsocketReceiveProtocol => "websocket_receive_protocol",
115+
Self::WebsocketReceiveWriteBufferFull => "websocket_receive_write_buffer_full",
116+
Self::WebsocketReceiveUtf8 => "websocket_receive_utf8",
117+
Self::WebsocketReceiveAttackAttempt => "websocket_receive_attack_attempt",
118+
Self::WebsocketReceiveUrl => "websocket_receive_url",
119+
Self::WebsocketReceiveHttp => "websocket_receive_http",
120+
Self::WebsocketReceiveHttpFormat => "websocket_receive_http_format",
73121
Self::WebsocketSendError => "websocket_send_error",
74122
Self::WebsocketStreamEnded => "websocket_stream_ended",
75123
Self::Unknown => "unknown",
@@ -196,7 +244,13 @@ metrics_group!(
196244
// Accepted-client disconnection `cause` label values are:
197245
// client_close, idle_timeout, incoming_queue_full, outgoing_queue_full,
198246
// module_exited, client_message_error, websocket_receive_error,
199-
// websocket_send_error, websocket_stream_ended, unknown.
247+
// websocket_receive_connection_closed, websocket_receive_already_closed,
248+
// websocket_receive_io, websocket_receive_tls, websocket_receive_capacity,
249+
// websocket_receive_protocol, websocket_receive_write_buffer_full,
250+
// websocket_receive_utf8, websocket_receive_attack_attempt,
251+
// websocket_receive_url, websocket_receive_http,
252+
// websocket_receive_http_format, websocket_send_error,
253+
// websocket_stream_ended, unknown.
200254
#[name = spacetime_worker_ws_client_disconnections_total]
201255
#[help = "The cumulative number of accepted websocket client disconnections by cause. Cause values are documented by ClientDisconnectCause::ALL."]
202256
#[labels(database_identity: Identity, cause: str)]
@@ -805,6 +859,15 @@ pub fn spawn_tokio_stats(node_id: String, rt_id: String, rt: tokio::runtime::Han
805859
#[cfg(test)]
806860
mod tests {
807861
use super::*;
862+
use std::collections::HashSet;
863+
864+
#[test]
865+
fn client_disconnect_cause_labels_are_unique() {
866+
let mut labels = HashSet::new();
867+
for cause in ClientDisconnectCause::ALL {
868+
assert!(labels.insert(cause.as_str()), "duplicate label for {cause:?}");
869+
}
870+
}
808871

809872
#[test]
810873
fn client_disconnect_recorder_records_only_first_cause() {

0 commit comments

Comments
 (0)