From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu, 27 May 2021 00:20:08 +0000 (+0000) Subject: Merge pull request #929 from jkczyz/2021-05-json-rpc-error X-Git-Tag: v0.0.98~16 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=8e7b5905fd84999318bdcf9cbcb9cfecbf58f532;hp=f0743433e7e1d302f79bec2fdb3f173560e4e954;p=rust-lightning 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 2e70e186..89eb95dc 100644 --- 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, +} + +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(body: MessageBody) -> Self { + fn responding_with_body(status: &str, body: MessageBody) -> 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(body: MessageBody) -> 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::("HTTP/1.1 404 Not Found", MessageBody::Empty) + } + + pub fn responding_with_server_error(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::("/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::().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 66570bcb..88199688 100644 --- 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::(&uri, &host, &self.basic_auth, content) - .await?.0; + let mut response = match self.client.post::(&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::() { + 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");