Expose remote cache read errors and fail on server disconnects

src/cache/cache.rs

@@ -213,6 +213,28 @@ impl RemoteStorage {
     }
 }
 
+#[cfg(any(
+    feature = "azure",
+    feature = "gcs",
+    feature = "gha",
+    feature = "memcached",
+    feature = "redis",
+    feature = "s3",
+    feature = "webdav",
+    feature = "oss",
+    feature = "cos"
+))]
+fn decode_remote_cache_read(result: opendal::Result<opendal::Buffer>) -> Result<Cache> {
+    match result {
+        Ok(res) => {
+            let hit = CacheRead::from(io::Cursor::new(res.to_bytes()))?;
+            Ok(Cache::Hit(hit))
+        }
+        Err(e) if e.kind() == opendal::ErrorKind::NotFound => Ok(Cache::Miss),
+        Err(e) => Err(e).context("failed to read remote cache entry"),
+    }
+}
+
 /// Implement storage for operator.
 #[cfg(any(
     feature = "azure",
@@ -228,17 +250,7 @@ impl RemoteStorage {
 #[async_trait]
 impl Storage for RemoteStorage {
     async fn get(&self, key: &str) -> Result<Cache> {
-        match self.operator.read(&normalize_key(key)).await {
-            Ok(res) => {
-                let hit = CacheRead::from(io::Cursor::new(res.to_bytes()))?;
-                Ok(Cache::Hit(hit))
-            }
-            Err(e) if e.kind() == opendal::ErrorKind::NotFound => Ok(Cache::Miss),
-            Err(e) => {
-                warn!("Got unexpected error: {:?}", e);
-                Ok(Cache::Miss)
-            }
-        }
+        decode_remote_cache_read(self.operator.read(&normalize_key(key)).await)
     }
 
     async fn put(&self, key: &str, entry: CacheWrite) -> Result<Duration> {
@@ -638,6 +650,26 @@ mod test {
     use crate::config::CacheModeConfig;
     use fs_err as fs;
 
+    #[cfg(feature = "s3")]
+    mod remote_storage {
+        use super::*;
+        use opendal::{Error, ErrorKind};
+
+        #[test]
+        fn get_propagates_unexpected_backend_errors() {
+            let error = decode_remote_cache_read(Err(Error::new(
+                ErrorKind::Unexpected,
+                "injected read failure",
+            )))
+            .unwrap_err();
+
+            assert_eq!(
+                error.downcast_ref::<Error>().map(Error::kind),
+                Some(ErrorKind::Unexpected)
+            );
+        }
+    }
+
     #[test]
     fn test_read_write_mode_local() {
         let runtime = tokio::runtime::Builder::new_current_thread()

src/commands.rs

@@ -507,8 +507,8 @@ fn handle_compile_finished(
 /// Handle `response`, the response from sending a `Compile` request to the server.
 ///
 /// If the server returned `CompileStarted`, reads the follow-up `CompileFinished`
-/// from `conn`, falling back to local execution if the server disconnects
-/// unexpectedly.  Delegates to `handle_compile_result` for the final dispatch.
+/// from `conn`. A read failure is returned by default; the explicit server-I/O
+/// fallback setting permits local execution instead.
 #[allow(clippy::too_many_arguments)]
 fn handle_compile_response<T>(
     creator: T,
@@ -532,25 +532,13 @@ where
                 Ok(Response::CompileFinished(result)) => Some(result),
                 Ok(_) => bail!("unexpected response from server"),
                 Err(e) => {
-                    match e.downcast_ref::<io::Error>() {
-                        Some(io_e) if io_e.kind() == io::ErrorKind::UnexpectedEof => {
-                            eprintln!(
-                                "sccache: warning: The server looks like it shut down \
-                                 unexpectedly, compiling locally instead"
-                            );
-                        }
-                        _ => {
-                            //TODO: something better here?
-                            if ignore_all_server_io_errors() {
-                                eprintln!(
-                                    "sccache: warning: error reading compile response from server \
-                                     compiling locally instead"
-                                );
-                            } else {
-                                return Err(e)
-                                    .context("error reading compile response from server");
-                            }
-                        }
+                    if ignore_all_server_io_errors() {
+                        eprintln!(
+                            "sccache: warning: error reading compile response from server; \
+                             compiling locally instead"
+                        );
+                    } else {
+                        return Err(e).context("error reading compile response from server");
                     }
                     None
                 }
@@ -586,7 +574,7 @@ where
             if let Some(finished) = finished {
                 return handle_compile_finished(finished, stdout, stderr);
             }
-            // Server disconnected before sending CompileFinished; fall back to local compilation.
+            // The explicit server-I/O fallback permits local compilation here.
         }
         CompileResponse::UnsupportedCompiler(s) => {
             debug!("Server sent UnsupportedCompiler: {:?}", s);
@@ -957,7 +945,9 @@ mod test {
         CommandCreator, ExitStatusValue, MockChild, MockCommandCreator, exit_status,
     };
     use crate::net::Connection;
+    use serial_test::serial;
     use std::sync::{Arc, Mutex};
+    use temp_env::{with_var, with_var_unset};
 
     fn make_runtime() -> Runtime {
         tokio::runtime::Builder::new_current_thread()
@@ -991,11 +981,7 @@ mod test {
         }
     }
 
-    /// A mid-compile server disconnect: the server sends CompileStarted then drops the
-    /// connection before CompileFinished.  handle_compile_response must fall back to
-    /// local compilation rather than panic.
-    #[test]
-    fn test_handle_compile_response_disconnect_falls_back_to_local() {
+    fn handle_disconnected_compile() -> Result<i32> {
         let mut runtime = make_runtime();
         let client = Client::new_num(1);
         let creator = Arc::new(Mutex::new(MockCommandCreator::new(&client)));
@@ -1018,7 +1004,7 @@ mod test {
         let mut stdout = vec![];
         let mut stderr = vec![];
 
-        let code = handle_compile_response(
+        handle_compile_response(
             creator,
             &mut runtime,
             &mut conn,
@@ -1029,6 +1015,33 @@ mod test {
             &mut stdout,
             &mut stderr,
         )
+    }
+
+    /// The default behavior documented in the README is to fail when the client loses
+    /// communication with the server.
+    #[test]
+    #[serial(server_io_error_env)]
+    fn test_handle_compile_response_disconnect_is_error_by_default() {
+        let error = with_var_unset(
+            "SCCACHE_IGNORE_SERVER_IO_ERROR",
+            handle_disconnected_compile,
+        )
+        .unwrap_err();
+
+        assert!(
+            format!("{error:#}").contains("error reading compile response from server"),
+            "unexpected error: {error:#}"
+        );
+    }
+
+    #[test]
+    #[serial(server_io_error_env)]
+    fn test_handle_compile_response_disconnect_can_fall_back_to_local() {
+        let code = with_var(
+            "SCCACHE_IGNORE_SERVER_IO_ERROR",
+            Some("1"),
+            handle_disconnected_compile,
+        )
         .unwrap();
 
         assert_eq!(code, 1);