From 4f66ebac06db7e6bb137d59594cd1b48486e989b Mon Sep 17 00:00:00 2001
From: Dan Glastonbury <dan.glastonbury@gmail.com>
Date: Wed, 25 Oct 2017 14:53:03 +1000
Subject: [PATCH] Bug 1410702 - P1: Handle errors in send_recv! r=kinetik

Connection handling code wasn't handling errors from receive()
properly. Attempting to unwrap on an Err was causing a panic.

MozReview-Commit-ID: GURe3FHPbjT

--HG--
extra : rebase_source : 664bf389020feb4a12f43422351c20ab7e804e30
---
 media/audioipc/audioipc/src/connection.rs |  3 +-
 media/audioipc/audioipc/src/messages.rs   |  4 +--
 media/audioipc/client/src/send_recv.rs    | 38 ++++++++++++++++-------
 media/audioipc/server/src/lib.rs          |  2 +-
 4 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/media/audioipc/audioipc/src/connection.rs b/media/audioipc/audioipc/src/connection.rs
index 4a956bdfebf0..27d332db86a1 100644
--- a/media/audioipc/audioipc/src/connection.rs
+++ b/media/audioipc/audioipc/src/connection.rs
@@ -9,6 +9,7 @@ use mio::unix::EventedFd;
 use serde::de::DeserializeOwned;
 use serde::ser::Serialize;
 use std::collections::VecDeque;
+use std::error::Error;
 use std::fmt::Debug;
 use std::io::{self, Read};
 use std::os::unix::io::{AsRawFd, RawFd};
@@ -126,7 +127,7 @@ impl Connection {
                 Ok(Async::NotReady) => bail!("Socket should be blocking."),
                 // TODO: Handle dropped message.
                 // Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => panic!("wouldblock"),
-                _ => bail!("socket write"),
+                Err(e) => bail!("stream recv_buf_fd returned: {}", e.description()),
             }
         }
     }
diff --git a/media/audioipc/audioipc/src/messages.rs b/media/audioipc/audioipc/src/messages.rs
index b5379080def8..cc7a087ab8f4 100644
--- a/media/audioipc/audioipc/src/messages.rs
+++ b/media/audioipc/audioipc/src/messages.rs
@@ -243,7 +243,5 @@ pub enum ClientMessage {
     StreamDataCallback(isize, usize),
     StreamStateCallback(ffi::cubeb_state),
 
-    ContextError(ffi::cubeb_error_code),
-    StreamError, /*(Error)*/
-    ClientError /*(Error)*/
+    Error(ffi::cubeb_error_code)
 }
diff --git a/media/audioipc/client/src/send_recv.rs b/media/audioipc/client/src/send_recv.rs
index 180a1207a96d..54ea659b581f 100644
--- a/media/audioipc/client/src/send_recv.rs
+++ b/media/audioipc/client/src/send_recv.rs
@@ -32,21 +32,35 @@ macro_rules! send_recv {
         }
     });
     (__recv $conn:expr, $rmsg:ident) => ({
-        let r = $conn.receive().unwrap();
-        if let ClientMessage::$rmsg = r {
-            Ok(())
-        } else {
-            debug!("receive error - got={:?}", r);
-            Err(ErrorCode::Error.into())
+        match $conn.receive() {
+            Ok(ClientMessage::$rmsg) => Ok(()),
+            // Macro can see ErrorCode but not Error? I don't understand.
+            // ::cubeb_core::Error is fragile but this isn't general purpose code.
+            Ok(ClientMessage::Error(e)) => Err(unsafe { ::cubeb_core::Error::from_raw(e) }),
+            Ok(m) => {
+                debug!("receive unexpected message - got={:?}", m);
+                Err(ErrorCode::Error.into())
+            },
+            Err(e) => {
+                debug!("receive error - got={:?}", e);
+                Err(ErrorCode::Error.into())
+            },
         }
     });
     (__recv $conn:expr, $rmsg:ident __result) => ({
-        let r = $conn.receive().unwrap();
-        if let ClientMessage::$rmsg(v) = r {
-            Ok(v)
-        } else {
-            debug!("receive error - got={:?}", r);
-            Err(ErrorCode::Error.into())
+        match $conn.receive() {
+            Ok(ClientMessage::$rmsg(v)) => Ok(v),
+            // Macro can see ErrorCode but not Error? I don't understand.
+            // ::cubeb_core::Error is fragile but this isn't general purpose code.
+            Ok(ClientMessage::Error(e)) => Err(unsafe { ::cubeb_core::Error::from_raw(e) }),
+            Ok(m) => {
+                debug!("receive unexpected message - got={:?}", m);
+                Err(ErrorCode::Error.into())
+            },
+            Err(e) => {
+                debug!("receive error - got={:?}", e);
+                Err(ErrorCode::Error.into())
+            },
         }
     })
 }
diff --git a/media/audioipc/server/src/lib.rs b/media/audioipc/server/src/lib.rs
index 39a1181e4af1..0be5504ff0e0 100644
--- a/media/audioipc/server/src/lib.rs
+++ b/media/audioipc/server/src/lib.rs
@@ -763,7 +763,7 @@ pub fn run(running: Arc<AtomicBool>) -> Result<()> {
 }
 
 fn error(error: cubeb::Error) -> ClientMessage {
-    ClientMessage::ContextError(error.raw_code())
+    ClientMessage::Error(error.raw_code())
 }
 
 struct ServerWrapper {