Merge pull request #929 from jkczyz/2021-05-json-rpc-error

Parse RPC errors as JSON content

diff --git a/lightning-block-sync/src/http.rs b/lightning-block-sync/src/http.rs
index 2e70e18659936e2746638ba684b36ce20c3095ca..89eb95dcff84a5165b5dcfdf27f54d71d69f57a4 100644 (file)
--- a/lightning-block-sync/src/http.rs
+++ b/lightning-block-sync/src/http.rs
@@ -5,6 +5,7 @@ use chunked_transfer;
 use serde_json;
 
 use std::convert::TryFrom;
+use std::fmt;
 #[cfg(not(feature = "tokio"))]
 use std::io::Write;
 use std::net::ToSocketAddrs;
@@ -348,21 +349,33 @@ impl HttpClient {
 
                if !status.is_ok() {
                        // TODO: Handle 3xx redirection responses.
-                       let error_details = match String::from_utf8(contents) {
-                               // Check that the string is all-ASCII with no control characters before returning
-                               // it.
-                               Ok(s) if s.as_bytes().iter().all(|c| c.is_ascii() && !c.is_ascii_control()) => s,
-                               _ => "binary".to_string()
+                       let error = HttpError {
+                               status_code: status.code.to_string(),
+                               contents,
                        };
-                       let error_msg = format!("Errored with status: {} and contents: {}",
-                                               status.code, error_details);
-                       return Err(std::io::Error::new(std::io::ErrorKind::Other, error_msg));
+                       return Err(std::io::Error::new(std::io::ErrorKind::Other, error));
                }
 
                Ok(contents)
        }
 }
 
+/// HTTP error consisting of a status code and body contents.
+#[derive(Debug)]
+pub(crate) struct HttpError {
+       pub(crate) status_code: String,
+       pub(crate) contents: Vec<u8>,
+}
+
+impl std::error::Error for HttpError {}
+
+impl fmt::Display for HttpError {
+       fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+               let contents = String::from_utf8_lossy(&self.contents);
+               write!(f, "status_code: {}, contents: {}", self.status_code, contents)
+       }
+}
+
 /// HTTP response status code as defined by [RFC 7231].
 ///
 /// [RFC 7231]: https://tools.ietf.org/html/rfc7231#section-6
@@ -538,16 +551,16 @@ pub(crate) mod client_tests {
        }
 
        impl HttpServer {
-               pub fn responding_with_ok<T: ToString>(body: MessageBody<T>) -> Self {
+               fn responding_with_body<T: ToString>(status: &str, body: MessageBody<T>) -> Self {
                        let response = match body {
-                               MessageBody::Empty => "HTTP/1.1 200 OK\r\n\r\n".to_string(),
+                               MessageBody::Empty => format!("{}\r\n\r\n", status),
                                MessageBody::Content(body) => {
                                        let body = body.to_string();
                                        format!(
-                                               "HTTP/1.1 200 OK\r\n\
+                                               "{}\r\n\
                                                 Content-Length: {}\r\n\
                                                 \r\n\
-                                                {}", body.len(), body)
+                                                {}", status, body.len(), body)
                                },
                                MessageBody::ChunkedContent(body) => {
                                        let mut chuncked_body = Vec::new();
@@ -557,18 +570,26 @@ pub(crate) mod client_tests {
                                                encoder.write_all(body.to_string().as_bytes()).unwrap();
                                        }
                                        format!(
-                                               "HTTP/1.1 200 OK\r\n\
+                                               "{}\r\n\
                                                 Transfer-Encoding: chunked\r\n\
                                                 \r\n\
-                                                {}", String::from_utf8(chuncked_body).unwrap())
+                                                {}", status, String::from_utf8(chuncked_body).unwrap())
                                },
                        };
                        HttpServer::responding_with(response)
                }
 
+               pub fn responding_with_ok<T: ToString>(body: MessageBody<T>) -> Self {
+                       HttpServer::responding_with_body("HTTP/1.1 200 OK", body)
+               }
+
                pub fn responding_with_not_found() -> Self {
-                       let response = "HTTP/1.1 404 Not Found\r\n\r\n".to_string();
-                       HttpServer::responding_with(response)
+                       HttpServer::responding_with_body::<String>("HTTP/1.1 404 Not Found", MessageBody::Empty)
+               }
+
+               pub fn responding_with_server_error<T: ToString>(content: T) -> Self {
+                       let body = MessageBody::Content(content);
+                       HttpServer::responding_with_body("HTTP/1.1 500 Internal Server Error", body)
                }
 
                fn responding_with(response: String) -> Self {
@@ -732,16 +753,15 @@ pub(crate) mod client_tests {
 
        #[tokio::test]
        async fn read_error() {
-               let response = String::from(
-                       "HTTP/1.1 500 Internal Server Error\r\n\
-                        Content-Length: 10\r\n\r\ntest error\r\n");
-               let server = HttpServer::responding_with(response);
+               let server = HttpServer::responding_with_server_error("foo");
 
                let mut client = HttpClient::connect(&server.endpoint()).unwrap();
                match client.get::<JsonResponse>("/foo", "foo.com").await {
                        Err(e) => {
-                               assert_eq!(e.get_ref().unwrap().to_string(), "Errored with status: 500 and contents: test error");
                                assert_eq!(e.kind(), std::io::ErrorKind::Other);
+                               let http_error = e.into_inner().unwrap().downcast::<HttpError>().unwrap();
+                               assert_eq!(http_error.status_code, "500");
+                               assert_eq!(http_error.contents, "foo".as_bytes());
                        },
                        Ok(_) => panic!("Expected error"),
                }

diff --git a/lightning-block-sync/src/rpc.rs b/lightning-block-sync/src/rpc.rs
index 66570bcb57c032353ab0d6a2f500bcda91740381..88199688aefd1a48fd128aac5c45f319f947c90e 100644 (file)
--- a/lightning-block-sync/src/rpc.rs
+++ b/lightning-block-sync/src/rpc.rs
@@ -2,7 +2,7 @@
 //! endpoint.
 
 use crate::{BlockHeaderData, BlockSource, AsyncBlockSourceResult};
-use crate::http::{HttpClient, HttpEndpoint, JsonResponse};
+use crate::http::{HttpClient, HttpEndpoint, HttpError, JsonResponse};
 
 use bitcoin::blockdata::block::Block;
 use bitcoin::hash_types::BlockHash;
@@ -47,8 +47,20 @@ impl RpcClient {
                        "id": &self.id.fetch_add(1, Ordering::AcqRel).to_string()
                });
 
-               let mut response = self.client.post::<JsonResponse>(&uri, &host, &self.basic_auth, content)
-                       .await?.0;
+               let mut response = match self.client.post::<JsonResponse>(&uri, &host, &self.basic_auth, content).await {
+                       Ok(JsonResponse(response)) => response,
+                       Err(e) if e.kind() == std::io::ErrorKind::Other => {
+                               match e.get_ref().unwrap().downcast_ref::<HttpError>() {
+                                       Some(http_error) => match JsonResponse::try_from(http_error.contents.clone()) {
+                                               Ok(JsonResponse(response)) => response,
+                                               Err(_) => Err(e)?,
+                                       },
+                                       None => Err(e)?,
+                               }
+                       },
+                       Err(e) => Err(e)?,
+               };
+
                if !response.is_object() {
                        return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "expected JSON object"));
                }
@@ -143,7 +155,7 @@ mod tests {
                let response = serde_json::json!({
                        "error": { "code": -8, "message": "invalid parameter" },
                });
-               let server = HttpServer::responding_with_ok(MessageBody::Content(response));
+               let server = HttpServer::responding_with_server_error(response);
                let mut client = RpcClient::new(CREDENTIALS, server.endpoint()).unwrap();
 
                let invalid_block_hash = serde_json::json!("foo");