From f447145478fc904f98a3e11558fc990ac3855ed0 Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 15 Feb 2023 12:01:57 -0500 Subject: [PATCH] Update websocket error messages to be more descriptive (#6298) * Pass the reason as the SyncError message * Updated changelog and read/write error messages * Updated changelog message * read_or_write_error message no longer optional * Updated changelog after release --- CHANGELOG.md | 1 + src/realm/sync/noinst/client_impl_base.cpp | 20 ++++++++++---------- src/realm/sync/noinst/client_impl_base.hpp | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2fcffb23c2..48890fcd3cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * `File::get_unique_id` now works on Windows * Replaced manual path string conversion with `std::filesystem::path` * Update yarn download path in install_baas.sh ([PR #6309](https://github.com/realm/realm-core/pull/6309)) +* Include the websocket close status reason when reporting errors to the sync client ([PR #6298](https://github.com/realm/realm-core/pull/6298)) ---------------------------------------------- diff --git a/src/realm/sync/noinst/client_impl_base.cpp b/src/realm/sync/noinst/client_impl_base.cpp index 91f56b4358b..29e7805e3c9 100644 --- a/src/realm/sync/noinst/client_impl_base.cpp +++ b/src/realm/sync/noinst/client_impl_base.cpp @@ -439,7 +439,7 @@ bool Connection::websocket_binary_message_received(util::Span data) std::error_code ec; using sf = SimulatedFailure; if (sf::trigger(sf::sync_client__read_head, ec)) { - read_or_write_error(ec); + read_or_write_error(ec, "simulated read error"); return bool(m_websocket); } @@ -472,7 +472,7 @@ bool Connection::websocket_closed_handler(bool was_clean, Status status) involuntary_disconnect(SessionErrorInfo{error_code, try_again}); // Throws } else if (status_code == ErrorCodes::ReadError || status_code == ErrorCodes::WriteError) { - read_or_write_error(error_code); + read_or_write_error(error_code, status.reason()); } else if (status_code == ErrorCodes::WebSocket_GoingAway || status_code == ErrorCodes::WebSocket_ProtocolError || status_code == ErrorCodes::WebSocket_UnsupportedData || status_code == ErrorCodes::WebSocket_Reserved || @@ -498,30 +498,30 @@ bool Connection::websocket_closed_handler(bool was_clean, Status status) error_code = ClientError::ssl_server_cert_rejected; constexpr bool is_fatal = true; m_reconnect_info.m_reason = ConnectionTerminationReason::ssl_certificate_rejected; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } else if (status_code == ErrorCodes::WebSocket_Client_Too_Old) { error_code = ClientError::client_too_old_for_server; constexpr bool is_fatal = true; m_reconnect_info.m_reason = ConnectionTerminationReason::http_response_says_fatal_error; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } else if (status_code == ErrorCodes::WebSocket_Client_Too_New) { error_code = ClientError::client_too_new_for_server; constexpr bool is_fatal = true; m_reconnect_info.m_reason = ConnectionTerminationReason::http_response_says_fatal_error; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } else if (status_code == ErrorCodes::WebSocket_Protocol_Mismatch) { error_code = ClientError::protocol_mismatch; constexpr bool is_fatal = true; m_reconnect_info.m_reason = ConnectionTerminationReason::http_response_says_fatal_error; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } else if (status_code == ErrorCodes::WebSocket_Fatal_Error || status_code == ErrorCodes::WebSocket_Forbidden) { constexpr bool is_fatal = true; m_reconnect_info.m_reason = ConnectionTerminationReason::http_response_says_fatal_error; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } else if (status_code == ErrorCodes::WebSocket_Unauthorized || status_code == ErrorCodes::WebSocket_MovedPermanently || @@ -530,7 +530,7 @@ bool Connection::websocket_closed_handler(bool was_clean, Status status) status_code == ErrorCodes::WebSocket_Retry_Error) { constexpr bool is_fatal = false; m_reconnect_info.m_reason = ConnectionTerminationReason::http_response_says_nonfatal_error; - close_due_to_client_side_error(error_code, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(error_code, status.reason(), is_fatal); // Throws } return bool(m_websocket); @@ -1153,11 +1153,11 @@ void Connection::handle_disconnect_wait(Status status) } -void Connection::read_or_write_error(std::error_code ec) +void Connection::read_or_write_error(std::error_code ec, std::string_view msg) { m_reconnect_info.m_reason = ConnectionTerminationReason::read_or_write_error; bool is_fatal = false; - close_due_to_client_side_error(ec, std::nullopt, is_fatal); // Throws + close_due_to_client_side_error(ec, msg, is_fatal); // Throws } diff --git a/src/realm/sync/noinst/client_impl_base.hpp b/src/realm/sync/noinst/client_impl_base.hpp index f3a08747a67..ba709a6e6ac 100644 --- a/src/realm/sync/noinst/client_impl_base.hpp +++ b/src/realm/sync/noinst/client_impl_base.hpp @@ -477,7 +477,7 @@ class ClientImpl::Connection { void handle_message_received(util::Span data); void initiate_disconnect_wait(); void handle_disconnect_wait(Status status); - void read_or_write_error(std::error_code); + void read_or_write_error(std::error_code ec, std::string_view msg); void close_due_to_protocol_error(std::error_code, std::optional msg = std::nullopt); void close_due_to_client_side_error(std::error_code, std::optional msg, bool is_fatal); void close_due_to_server_side_error(ProtocolError, const ProtocolErrorInfo& info);