From 3644c526304677652b8cc175253ce090cf015a41 Mon Sep 17 00:00:00 2001
From: Pete Hayes <pete@hayes.id.au>
Date: Tue, 24 Oct 2017 12:55:02 +0100
Subject: [PATCH] Better API error handling for Agent

---
 agent/src/errors.rs |  4 ++--
 agent/src/main.rs   | 35 +++++++++++++++++++++++++----------
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/agent/src/errors.rs b/agent/src/errors.rs
index c297fbb..9c3b6ef 100644
--- a/agent/src/errors.rs
+++ b/agent/src/errors.rs
@@ -4,6 +4,7 @@
 // https://www.tldrlegal.com/l/mpl-2.0>. This file may not be copied,
 // modified, or distributed except according to those terms.
 
+use error_chain::ChainedError;
 use futures::Future;
 use intecture_api;
 use std::{convert, error, io};
@@ -16,8 +17,7 @@ error_chain! {
 
 impl convert::From<Error> for io::Error {
     fn from(e: Error) -> io::Error {
-        // @todo Return whole error chain
-        io::Error::new(io::ErrorKind::Other, e.description())
+        io::Error::new(io::ErrorKind::Other, format!("{}", e.display_chain()))
     }
 }
 
diff --git a/agent/src/main.rs b/agent/src/main.rs
index 29d34e8..134fce2 100644
--- a/agent/src/main.rs
+++ b/agent/src/main.rs
@@ -18,11 +18,12 @@ extern crate toml;
 
 mod errors;
 
+use error_chain::ChainedError;
 use errors::*;
 use futures::{future, Future};
 use intecture_api::host::local::Local;
 use intecture_api::host::remote::{JsonLineProto, LineMessage};
-use intecture_api::remote::{Executable, Request};
+use intecture_api::remote::{Executable, Request, ResponseResult};
 use std::fs::File;
 use std::io::{self, Read};
 use std::net::SocketAddr;
@@ -49,9 +50,9 @@ impl Service for Api {
             Message::WithoutBody(req) => req,
         };
 
-        let request: Request = match serde_json::from_value(req).chain_err(|| "Received invalid Request") {
+        let request: Request = match serde_json::from_value(req).chain_err(|| "Could not deserialize Request") {
             Ok(r) => r,
-            Err(e) => return Box::new(future::err(e))
+            Err(e) => return Box::new(future::ok(error_to_msg(e))),
         };
 
         // XXX Danger zone! If we're running multiple threads, this `unwrap()`
@@ -62,14 +63,19 @@ impl Service for Api {
         let handle = self.remote.handle().unwrap();
         Box::new(request.exec(&self.host, &handle)
             .chain_err(|| "Failed to execute Request")
-            .and_then(|mut msg| {
-                let body = msg.take_body();
-                match serde_json::to_value(msg.into_inner()).chain_err(|| "Could not serialize result") {
-                    Ok(v) => match body {
-                        Some(b) => future::ok(Message::WithBody(v, b)),
-                        None => future::ok(Message::WithoutBody(v)),
+            .then(|req| {
+                match req {
+                    Ok(mut msg) => {
+                        let body = msg.take_body();
+                        match serde_json::to_value(msg.into_inner()).chain_err(|| "Could not serialize Result") {
+                            Ok(v) => match body {
+                                Some(b) => future::ok(Message::WithBody(v, b)),
+                                None => future::ok(Message::WithoutBody(v)),
+                            },
+                            Err(e) => future::ok(error_to_msg(e)),
+                        }
                     },
-                    Err(e) => future::err(e),
+                    Err(e) => future::ok(error_to_msg(e)),
                 }
             }))
     }
@@ -143,3 +149,12 @@ quick_main!(|| -> Result<()> {
     });
     Ok(())
 });
+
+fn error_to_msg(e: Error) -> LineMessage {
+    let response = ResponseResult::Err(format!("{}", e.display_chain()));
+    // If we can't serialize this, we can't serialize anything, so
+    // panicking is appropriate.
+    let value = serde_json::to_value(response)
+        .expect("Cannot serialize ResponseResult::Err. This is bad...");
+    Message::WithoutBody(value)
+}