Fix unnecessary re_viewer dependency (#6369)

### What

The mechanism for identifying Rerun clients was implemented in #6204.
However, during the release I accidentally "fixed" the build of `cargo
build -p re_viewer --no-default-features` twice:
* The seemingly correct fix was to depend on `re_sdk_comms` with the
`server` feature always enabled.
* The definitely wrong thing that slipped through is to feature guard
the section that needs the `re_sdk_comms` server errors with
`#[cfg(feature = "server")]`. But `re_viewer` doesn't have a `server`
feature, disabling this effectively. This was reverted in
#6368

It's up to users of `re_viewer` to allow the serve feature, making
`re_viewer` agonistic of the presence of this error type in its smart
channel. Therefore I now moved out the error type to `re_sdk_comms`
lib.rs so that `re_viewer` doesn't have to depend on the
`re_sdk_comms`'s server feature.


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6369?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6369?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6369)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

CHANGELOG.md

@@ -87,6 +87,7 @@ As always there's a lot going on under the hood:
 - New data APIs 14: port everything that used to be uncached [#6035](https://github.com/rerun-io/rerun/pull/6035)
 - Make visible time range ui aware of latest-at & `QueryRange` [#6176](https://github.com/rerun-io/rerun/pull/6176)
 - Visible time ranges are now specified per timeline, not per timeline type [#6204](https://github.com/rerun-io/rerun/pull/6204)
+- Send TCP protocol header to ignore non-rerun clients [#6253](https://github.com/rerun-io/rerun/pull/6253) (thanks [@gurry](https://github.com/gurry)!)
 
 #### 🚀 Performance Improvements
 - New data APIs 4: cached latest-at mono helpers everywhere [#5606](https://github.com/rerun-io/rerun/pull/5606)
@@ -168,7 +169,6 @@ As always there's a lot going on under the hood:
 - New data APIs 8: uncached range queries [#5687](https://github.com/rerun-io/rerun/pull/5687)
 - New data APIs 10: stats and debug tools for new caches [#5990](https://github.com/rerun-io/rerun/pull/5990)
 - Validate the blueprint schema when we try to activate a blueprint sent from SDK [#6283](https://github.com/rerun-io/rerun/pull/6283)
-- Send TCP protocol header to ignore non-rerun clients [#6253](https://github.com/rerun-io/rerun/pull/6253) (thanks [@gurry](https://github.com/gurry)!)
 
 
 ## [0.15.1](https://github.com/rerun-io/rerun/compare/0.15.0...0.15.1) - Bug fix for notebooks - 2024-04-11

crates/re_sdk_comms/src/lib.rs

@@ -17,7 +17,47 @@ pub use {buffered_client::Client, tcp_client::ClientError};
 mod server;
 
 #[cfg(feature = "server")]
-pub use server::{serve, ConnectionError, ServerError, ServerOptions};
+pub use server::{serve, ServerError, ServerOptions};
+
+/// Server connection error.
+///
+/// This can only occur when using the `server` feature,
+/// However it is defined here so that crates that want to react to this error can do so without
+/// needing to depend on the `server` feature directly.
+/// This is useful when processing errors from a passed-in `re_smart_channel` channel as done by `re_viewer` as of writing.
+#[derive(thiserror::Error, Debug)]
+pub enum ConnectionError {
+    #[error("An unknown client tried to connect")]
+    UnknownClient,
+
+    #[error(transparent)]
+    VersionError(#[from] VersionError),
+
+    #[error(transparent)]
+    SendError(#[from] std::io::Error),
+
+    #[error(transparent)]
+    DecodeError(#[from] re_log_encoding::decoder::DecodeError),
+
+    #[error("The receiving end of the channel was closed")]
+    ChannelDisconnected(#[from] re_smart_channel::SendError<re_log_types::LogMsg>),
+}
+
+#[derive(thiserror::Error, Debug)]
+#[allow(unused)]
+pub enum VersionError {
+    #[error("SDK client is using an older protocol version ({client_version}) than the SDK server ({server_version})")]
+    ClientIsOlder {
+        client_version: u16,
+        server_version: u16,
+    },
+
+    #[error("SDK client is using a newer protocol version ({client_version}) than the SDK server ({server_version})")]
+    ClientIsNewer {
+        client_version: u16,
+        server_version: u16,
+    },
+}
 
 pub const PROTOCOL_VERSION_0: u16 = 0;
 

crates/re_sdk_comms/src/server.rs

@@ -9,6 +9,8 @@ use rand::{Rng as _, SeedableRng};
 use re_log_types::{LogMsg, TimePoint, TimeType, TimelineName};
 use re_smart_channel::{Receiver, Sender};
 
+use crate::{ConnectionError, VersionError};
+
 #[derive(thiserror::Error, Debug)]
 pub enum ServerError {
     #[error("Failed to bind TCP address {bind_addr:?}. Another Rerun instance is probably running. {err}")]
@@ -21,39 +23,6 @@ pub enum ServerError {
     FailedToSpawnThread(#[from] std::io::Error),
 }
 
-#[derive(thiserror::Error, Debug)]
-pub enum VersionError {
-    #[error("SDK client is using an older protocol version ({client_version}) than the SDK server ({server_version})")]
-    ClientIsOlder {
-        client_version: u16,
-        server_version: u16,
-    },
-
-    #[error("SDK client is using a newer protocol version ({client_version}) than the SDK server ({server_version})")]
-    ClientIsNewer {
-        client_version: u16,
-        server_version: u16,
-    },
-}
-
-#[derive(thiserror::Error, Debug)]
-pub enum ConnectionError {
-    #[error("An unknown client tried to connect")]
-    UnknownClient,
-
-    #[error(transparent)]
-    VersionError(#[from] VersionError),
-
-    #[error(transparent)]
-    SendError(#[from] std::io::Error),
-
-    #[error(transparent)]
-    DecodeError(#[from] re_log_encoding::decoder::DecodeError),
-
-    #[error("The receiving end of the channel was closed")]
-    ChannelDisconnected(#[from] re_smart_channel::SendError<LogMsg>),
-}
-
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct ServerOptions {
     /// If the latency in the [`LogMsg`] channel is greater than this,

crates/re_viewer/Cargo.toml

@@ -56,7 +56,7 @@ re_log_types.workspace = true
 re_memory.workspace = true
 re_query.workspace = true
 re_renderer = { workspace = true, default-features = false }
-re_sdk_comms = { workspace = true, features = ["server"] }
+re_sdk_comms.workspace = true
 re_smart_channel.workspace = true
 re_space_view.workspace = true
 re_space_view_bar_chart.workspace = true

crates/re_viewer/src/app.rs

@@ -989,8 +989,9 @@ impl App {
                                 matches!(e, re_sdk_comms::ConnectionError::UnknownClient)
                             })
                         {
-                            // An unknown client that probably stumbled onto the wrong port.
-                            // Don't log as an error (https://github.com/rerun-io/rerun/issues/5883).
+                            // This can happen if a client tried to connect but didn't send the `re_sdk_comms::PROTOCOL_HEADER`.
+                            // Likely an unknown client stumbled onto the wrong port - don't log as an error.
+                            // (for more information see https://github.com/rerun-io/rerun/issues/5883).
                             re_log::debug!("{log_msg}");
                             continue;
                         }