Make query_json_api return a QueryApiJsonResult instead of a String, to allow chunked serialization in the caller.

This lets us overlap the JSON serialization with the compression of the
HTTP response: https://share.firefox.dev/46AS9jZ

Author: mstange

samply-api/src/asm/mod.rs

@@ -5,21 +5,17 @@ use samply_symbols::{
     object, CodeByteReadingError, CodeId, FileAndPathHelper, FileAndPathHelperError, LibraryInfo,
     LookupAddress, SymbolManager,
 };
-use serde_json::json;
 use yaxpeax_arch::{Arch, DecodeError, LengthedInstruction, Reader, U8Reader};
 use yaxpeax_x86::amd64::{Opcode, Operand};
 
 use self::response_json::Response;
 use crate::asm::response_json::DecodedInstruction;
 
-mod request_json;
-mod response_json;
+pub mod request_json;
+pub mod response_json;
 
 #[derive(thiserror::Error, Debug)]
-enum AsmError {
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
-
+pub enum AsmError {
     #[error("An error occurred when loading the binary: {0}")]
     LoadBinaryError(#[from] samply_symbols::Error),
 
@@ -49,20 +45,15 @@ impl<'a, H: FileAndPathHelper> AsmApi<'a, H> {
         Self { symbol_manager }
     }
 
-    pub async fn query_api_json(&self, request_json: &str) -> String {
-        match self.query_api_fallible_json(request_json).await {
-            Ok(response_json) => response_json,
-            Err(err) => json!({ "error": err.to_string() }).to_string(),
-        }
-    }
-
-    async fn query_api_fallible_json(&self, request_json: &str) -> Result<String, AsmError> {
+    pub async fn query_api_json(
+        &self,
+        request_json: &str,
+    ) -> Result<response_json::Response, crate::Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
-        let response = self.query_api(&request).await?;
-        Ok(serde_json::to_string(&response)?)
+        Ok(self.query_api(&request).await?)
     }
 
-    async fn query_api(
+    pub async fn query_api(
         &self,
         request: &request_json::Request,
     ) -> Result<response_json::Response, AsmError> {

samply-api/src/error.rs

@@ -1,17 +1,44 @@
+use serde::Serialize;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum Error {
+    #[error("Unrecognized URL: {0}")]
+    UnrecognizedUrl(String),
+
+    #[error("Couldn't parse request: {0}")]
+    ParseRequestErrorSerde(#[from] serde_json::error::Error),
+
+    #[error("Malformed request JSON: {0}")]
+    ParseRequestErrorContents(&'static str),
+
     #[error("{0}")]
     Symbols(
         #[from]
         #[source]
         samply_symbols::Error,
     ),
 
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
+    #[error("{0}")]
+    Source(
+        #[from]
+        #[source]
+        super::source::SourceError,
+    ),
 
-    #[error("Malformed request JSON: {0}")]
-    ParseRequestErrorContents(&'static str),
+    #[error("{0}")]
+    Asm(
+        #[from]
+        #[source]
+        super::asm::AsmError,
+    ),
+}
+
+impl Serialize for Error {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.to_string().serialize(serializer)
+    }
 }

samply-api/src/lib.rs

@@ -28,7 +28,7 @@
 //!     let symbol_manager = SymbolManager::with_helper(helper);
 //!     let api = samply_api::Api::new(&symbol_manager);
 //!
-//!     api.query_api(
+//!     let api_result = api.query_api(
 //!         "/symbolicate/v5",
 //!         r#"{
 //!             "memoryMap": [
@@ -46,7 +46,8 @@
 //!               ]
 //!             ]
 //!           }"#,
-//!     ).await
+//!     ).await;
+//!     serde_json::to_string(&api_result).unwrap()
 //! }
 //!
 //! struct ExampleHelper {
@@ -147,7 +148,6 @@ use debugid::DebugId;
 pub use samply_symbols;
 pub use samply_symbols::debugid;
 use samply_symbols::{FileAndPathHelper, SymbolManager};
-use serde_json::json;
 use source::SourceApi;
 use symbolicate::SymbolicateApi;
 
@@ -158,6 +158,8 @@ mod hex;
 mod source;
 mod symbolicate;
 
+pub use error::Error;
+
 pub(crate) fn to_debug_id(breakpad_id: &str) -> Result<DebugId, samply_symbols::Error> {
     // Only accept breakpad IDs with the right syntax, and which aren't all-zeros.
     match DebugId::from_breakpad(breakpad_id) {
@@ -168,6 +170,42 @@ pub(crate) fn to_debug_id(breakpad_id: &str) -> Result<DebugId, samply_symbols::
     }
 }
 
+#[derive(serde_derive::Serialize)]
+pub struct QueryApiJsonResult(QueryApiJsonResultInner);
+
+#[derive(serde_derive::Serialize)]
+#[serde(untagged)]
+enum QueryApiJsonResultInner {
+    SymbolicateResponse(symbolicate::response_json::Response),
+    SourceResponse(source::response_json::Response),
+    AsmResponse(asm::response_json::Response),
+    Err(Error),
+}
+
+impl From<symbolicate::response_json::Response> for QueryApiJsonResult {
+    fn from(value: symbolicate::response_json::Response) -> Self {
+        Self(QueryApiJsonResultInner::SymbolicateResponse(value))
+    }
+}
+
+impl From<source::response_json::Response> for QueryApiJsonResult {
+    fn from(value: source::response_json::Response) -> Self {
+        Self(QueryApiJsonResultInner::SourceResponse(value))
+    }
+}
+
+impl From<asm::response_json::Response> for QueryApiJsonResult {
+    fn from(value: asm::response_json::Response) -> Self {
+        Self(QueryApiJsonResultInner::AsmResponse(value))
+    }
+}
+
+impl From<Error> for QueryApiJsonResult {
+    fn from(value: Error) -> Self {
+        Self(QueryApiJsonResultInner::Err(value))
+    }
+}
+
 #[derive(Clone, Copy)]
 pub struct Api<'a, H: FileAndPathHelper> {
     symbol_manager: &'a SymbolManager<H>,
@@ -194,18 +232,30 @@ impl<'a, H: FileAndPathHelper> Api<'a, H> {
     ///    symbol information for that address.
     ///  - `/asm/v1`: Experimental API. Symbolicates an address and lets you read one of the files in the
     ///    symbol information for that address.
-    pub async fn query_api(self, request_url: &str, request_json_data: &str) -> String {
+    pub async fn query_api(self, request_url: &str, request_json_data: &str) -> QueryApiJsonResult {
+        self.query_api_fallible(request_url, request_json_data)
+            .await
+            .unwrap_or_else(|e| e.into())
+    }
+    pub async fn query_api_fallible(
+        self,
+        request_url: &str,
+        request_json_data: &str,
+    ) -> Result<QueryApiJsonResult, Error> {
         if request_url == "/symbolicate/v5" {
             let symbolicate_api = SymbolicateApi::new(self.symbol_manager);
-            symbolicate_api.query_api_json(request_json_data).await
+            Ok(symbolicate_api
+                .query_api_json(request_json_data)
+                .await?
+                .into())
         } else if request_url == "/source/v1" {
             let source_api = SourceApi::new(self.symbol_manager);
-            source_api.query_api_json(request_json_data).await
+            Ok(source_api.query_api_json(request_json_data).await?.into())
         } else if request_url == "/asm/v1" {
             let asm_api = AsmApi::new(self.symbol_manager);
-            asm_api.query_api_json(request_json_data).await
+            Ok(asm_api.query_api_json(request_json_data).await?.into())
         } else {
-            json!({ "error": format!("Unrecognized URL {request_url}") }).to_string()
+            Err(Error::UnrecognizedUrl(request_url.into()))
         }
     }
 }

samply-api/src/source/mod.rs

@@ -1,19 +1,15 @@
 use samply_symbols::{
     FileAndPathHelper, FileAndPathHelperError, LibraryInfo, LookupAddress, SymbolManager,
 };
-use serde_json::json;
 
 use crate::api_file_path::to_api_file_path;
-use crate::to_debug_id;
+use crate::{to_debug_id, Error};
 
-mod request_json;
-mod response_json;
+pub mod request_json;
+pub mod response_json;
 
 #[derive(thiserror::Error, Debug)]
-enum SourceError {
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
-
+pub enum SourceError {
     #[error("Could not obtain symbols for the requested library: {0}")]
     NoSymbols(#[from] samply_symbols::Error),
 
@@ -37,20 +33,15 @@ impl<'a, H: FileAndPathHelper> SourceApi<'a, H> {
         Self { symbol_manager }
     }
 
-    pub async fn query_api_json(&self, request_json: &str) -> String {
-        match self.query_api_fallible_json(request_json).await {
-            Ok(response_json) => response_json,
-            Err(err) => json!({ "error": err.to_string() }).to_string(),
-        }
-    }
-
-    async fn query_api_fallible_json(&self, request_json: &str) -> Result<String, SourceError> {
+    pub async fn query_api_json(
+        &self,
+        request_json: &str,
+    ) -> Result<response_json::Response, Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
-        let response = self.query_api(&request).await?;
-        Ok(serde_json::to_string(&response)?)
+        Ok(self.query_api(&request).await?)
     }
 
-    async fn query_api(
+    pub async fn query_api(
         &self,
         request: &request_json::Request,
     ) -> Result<response_json::Response, SourceError> {

samply-api/src/symbolicate/mod.rs

@@ -13,7 +13,6 @@ pub mod response_json;
 
 use looked_up_addresses::LookedUpAddresses;
 use request_json::Lib;
-use serde_json::json;
 
 pub struct SymbolicateApi<'a, H: FileAndPathHelper> {
     symbol_manager: &'a SymbolManager<H>,
@@ -25,17 +24,12 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
         Self { symbol_manager }
     }
 
-    pub async fn query_api_json(&self, request_json: &str) -> String {
-        match self.query_api_fallible_json(request_json).await {
-            Ok(response_json) => response_json,
-            Err(err) => json!({ "error": err.to_string() }).to_string(),
-        }
-    }
-
-    pub async fn query_api_fallible_json(&self, request_json: &str) -> Result<String, Error> {
+    pub async fn query_api_json(
+        &self,
+        request_json: &str,
+    ) -> Result<response_json::Response, Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
-        let response = self.query_api(request).await?;
-        Ok(serde_json::to_string(&response)?)
+        self.query_api(request).await
     }
 
     pub async fn query_api(

samply-api/tests/integration_tests/main.rs

@@ -4,18 +4,23 @@ use std::path::PathBuf;
 
 use assert_json_diff::assert_json_eq;
 pub use samply_api::debugid::DebugId;
-use samply_api::{samply_symbols, Api};
+use samply_api::{samply_symbols, Api, QueryApiJsonResult};
 use samply_symbols::{
     CandidatePathInfo, FileAndPathHelper, FileAndPathHelperResult, FileLocation, LibraryInfo,
     OptionallySendFuture, SymbolManager,
 };
 
-pub async fn query_api(request_url: &str, request_json: &str, symbol_directory: PathBuf) -> String {
+pub async fn query_api(
+    request_url: &str,
+    request_json: &str,
+    symbol_directory: PathBuf,
+) -> QueryApiJsonResult {
     let helper = Helper { symbol_directory };
     let symbol_manager = SymbolManager::with_helper(helper);
     let api = Api::new(&symbol_manager);
     api.query_api(request_url, request_json).await
 }
+
 struct Helper {
     symbol_directory: PathBuf,
 }
@@ -229,6 +234,7 @@ fn compare_snapshot(
         request_json,
         symbol_directory,
     ));
+    let output = serde_json::to_string(&output).unwrap();
 
     let output_json: serde_json::Value = serde_json::from_str(&output).unwrap();
 

samply/src/server.rs

@@ -1,5 +1,7 @@
 use std::collections::HashMap;
+use std::convert::Infallible;
 use std::ffi::OsStr;
+use std::io::BufWriter;
 use std::net::{IpAddr, SocketAddr};
 use std::ops::Range;
 use std::path::{Path, PathBuf};
@@ -8,7 +10,7 @@ use std::sync::Arc;
 
 use futures_util::TryStreamExt;
 use http_body_util::combinators::BoxBody;
-use http_body_util::{BodyExt, Either, StreamBody};
+use http_body_util::{BodyExt, Either, Full, StreamBody};
 use hyper::body::{Bytes, Frame};
 use hyper::server::conn::http1;
 use hyper::service::service_fn;
@@ -240,13 +242,15 @@ async fn run_server(
     }
 }
 
+type MyBody = Either<String, Either<BoxBody<Bytes, std::io::Error>, BoxBody<Bytes, Infallible>>>;
+
 async fn symbolication_service(
     req: Request<hyper::body::Incoming>,
     template_values: Arc<HashMap<&'static str, String>>,
     symbol_manager: Arc<SymbolManager>,
     profile_filename: Option<PathBuf>,
     path_prefix: String,
-) -> Result<Response<Either<String, BoxBody<Bytes, std::io::Error>>>, hyper::Error> {
+) -> Result<Response<MyBody>, hyper::Error> {
     let has_profile = profile_filename.is_some();
     let method = req.method();
     let path = req.uri().path();
@@ -339,7 +343,7 @@ async fn symbolication_service(
             let reader_stream = ReaderStream::new(reader);
 
             let stream_body = StreamBody::new(reader_stream.map_ok(Frame::data));
-            *response.body_mut() = Either::Right(stream_body.boxed());
+            *response.body_mut() = Either::Right(Either::Left(stream_body.boxed()));
         }
         (&Method::POST, path, _) => {
             response.headers_mut().insert(
@@ -348,13 +352,17 @@ async fn symbolication_service(
             );
             let path = path.to_string();
             // Await the full body to be concatenated into a `Collected<Bytes>`.
-            let full_body = req.into_body().collect().await?;
+            let request_body = req.into_body().collect().await?;
             // Convert the `Collected<Bytes>` into a `String`.
-            let full_body =
-                String::from_utf8(full_body.to_bytes().to_vec()).expect("invalid utf-8");
-            let response_json = symbol_manager.query_json_api(&path, &full_body).await;
-
-            *response.body_mut() = Either::Left(response_json);
+            let request_body =
+                String::from_utf8(request_body.to_bytes().to_vec()).expect("invalid utf-8");
+            let response_json = symbol_manager.query_json_api(&path, &request_body).await;
+            let mut response_bytes = Vec::new();
+            let response_writer = BufWriter::new(&mut response_bytes);
+            serde_json::to_writer(response_writer, &response_json).expect("json writing error");
+            let response_body = Full::new(Bytes::from(response_bytes));
+
+            *response.body_mut() = Either::Right(Either::Right(response_body.boxed()));
         }
         _ => {
             *response.status_mut() = StatusCode::NOT_FOUND;