From 52f3565a6f92bcaa16e49f6879d263174fe74084 Mon Sep 17 00:00:00 2001 From: Angel M De Miguel Date: Fri, 5 May 2023 13:51:51 +0200 Subject: [PATCH 1/3] feat: allow configuring stderr in the server / worker --- crates/server/src/handlers/worker.rs | 5 +-- crates/server/src/lib.rs | 18 +++++++++- crates/worker/src/lib.rs | 40 ++++++++++----------- crates/worker/src/stdio.rs | 53 ++++++++++++++++++++++++++++ src/main.rs | 2 +- 5 files changed, 94 insertions(+), 24 deletions(-) create mode 100644 crates/worker/src/stdio.rs diff --git a/crates/server/src/handlers/worker.rs b/crates/server/src/handlers/worker.rs index 6f3dd199..f6559aac 100644 --- a/crates/server/src/handlers/worker.rs +++ b/crates/server/src/handlers/worker.rs @@ -8,7 +8,7 @@ use actix_web::{ web::{Bytes, Data}, HttpRequest, HttpResponse, }; -use std::sync::RwLock; +use std::{sync::RwLock, fs::File}; use wws_router::Routes; use wws_worker::io::WasmOutput; @@ -31,6 +31,7 @@ use wws_worker::io::WasmOutput; /// allowing Actix to select it for us. pub async fn handle_worker(req: HttpRequest, body: Bytes) -> HttpResponse { let routes = req.app_data::>().unwrap(); + let stderr_file = req.app_data::>>().unwrap(); let data_connectors = req .app_data::>>() .unwrap() @@ -68,7 +69,7 @@ pub async fn handle_worker(req: HttpRequest, body: Bytes) -> HttpResponse { None => None, }; - let (handler_result, handler_success) = match route.worker.run(&req, &body_str, store, vars) + let (handler_result, handler_success) = match route.worker.run(&req, &body_str, store, vars, stderr_file.get_ref()) { Ok(output) => (output, true), Err(_) => (WasmOutput::failed(), false), diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 86d867fa..711504f4 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -14,6 +14,7 @@ use anyhow::Result; use handlers::assets::handle_assets; use handlers::not_found::handle_not_found; use handlers::worker::handle_worker; +use std::fs::OpenOptions; use std::path::Path; use std::sync::RwLock; use wws_data_kv::KV; @@ -32,11 +33,25 @@ pub async fn serve( base_routes: Routes, hostname: &str, port: u16, + stderr: Option<&Path> ) -> Result { // Initializes the data connectors. For now, just KV let data = Data::new(RwLock::new(DataConnectors::default())); let routes = Data::new(base_routes); let root_path = Data::new(root_path.to_owned()); + let stderr_file; + + // Configure stderr + if let Some(path) = stderr { + let file = OpenOptions::new() + .read(true) + .write(true) + .open(path)?; + + stderr_file = Data::new(Some(file)); + } else { + stderr_file = Data::new(None); + } let server = HttpServer::new(move || { let mut app = App::new() @@ -46,7 +61,8 @@ pub async fn serve( .wrap(middleware::NormalizePath::trim()) .app_data(Data::clone(&routes)) .app_data(Data::clone(&data)) - .app_data(Data::clone(&root_path)); + .app_data(Data::clone(&root_path)) + .app_data(Data::clone(&stderr_file)); // Append routes to the current service for route in routes.routes.iter() { diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 2853d6a7..730c6a42 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -3,15 +3,16 @@ pub mod config; pub mod io; +mod stdio; use actix_web::HttpRequest; use anyhow::{anyhow, Result}; use config::Config; use io::{WasmInput, WasmOutput}; -use std::fs; +use stdio::Stdio; +use std::fs::{self, File}; use std::path::PathBuf; use std::{collections::HashMap, path::Path}; -use wasi_common::pipe::{ReadPipe, WritePipe}; use wasmtime::{Engine, Linker, Module, Store}; use wasmtime_wasi::{Dir, WasiCtxBuilder}; use wws_config::Config as ProjectConfig; @@ -72,13 +73,21 @@ impl Worker { body: &str, kv: Option>, vars: &HashMap, + stderr: &Option ) -> Result { let input = serde_json::to_string(&WasmInput::new(request, body, kv)).unwrap(); - // Prepare STDIO - let stdout = WritePipe::new_in_memory(); - let stderr = WritePipe::new_in_memory(); - let stdin = ReadPipe::from(input); + // Prepare the stderr file if present + let stderr_file; + + if let Some(file) = stderr { + stderr_file = Some(file.try_clone()?); + } else { + stderr_file = None; + } + + // Initialize stdio and configure it + let stdio = Stdio::new(&input, stderr_file); let mut linker = Linker::new(&self.engine); wasmtime_wasi::add_to_linker(&mut linker, |s| s)?; @@ -89,11 +98,11 @@ impl Worker { // Create the initial WASI context let mut wasi_builder = WasiCtxBuilder::new() - .stdin(Box::new(stdin)) - .stdout(Box::new(stdout.clone())) - .stderr(Box::new(stderr.clone())) .envs(&tuple_vars)?; + // Configure the stdio + wasi_builder = stdio.configure_wasi_ctx(wasi_builder); + // Mount folders from the configuration if let Some(folders) = self.config.folders.as_ref() { for folder in folders { @@ -122,17 +131,8 @@ impl Worker { drop(store); - let err_contents: Vec = stderr - .try_into_inner() - .map_err(|_err| anyhow::Error::msg("Nothing to show"))? - .into_inner(); - - let string_err = String::from_utf8(err_contents)?; - if !string_err.is_empty() { - println!("Error: {string_err}"); - } - - let contents: Vec = stdout + let contents: Vec = stdio + .stdout .try_into_inner() .map_err(|_err| anyhow::Error::msg("Nothing to show"))? .into_inner(); diff --git a/crates/worker/src/stdio.rs b/crates/worker/src/stdio.rs new file mode 100644 index 00000000..bc451eb5 --- /dev/null +++ b/crates/worker/src/stdio.rs @@ -0,0 +1,53 @@ +use std::{io::Cursor, fs::{File}}; +use wasi_common::pipe::{ReadPipe, WritePipe}; +use wasmtime_wasi::WasiCtxBuilder; + +/// A library to configure the stdio of the WASI context. +/// Note that currently, wws relies on stdin and stdout +/// to send and read data from the worker. +/// +/// The stderr is configurable just to cover use cases in which +/// wws is used as a library and we want to expose the logs +/// +/// @see https://github.com/vmware-labs/wasm-workers-server/issues/125 +/// +/// The stdin/stdout approach will change in the future with +/// a more performant and appropiate way. +pub struct Stdio { + /// Defines the stdin ReadPipe to send the data to the module + pub stdin: ReadPipe>, + /// Defines the stdout to extract the data from the module + pub stdout: WritePipe>>, + /// Defines the stderr to expose logs from inside the module + pub stderr: Option> +} + +impl Stdio { + /// Initialize the stdio. The stdin will contain the input data. + pub fn new(input: &str, stderr_file: Option) -> Self { + let stderr; + + if let Some(file) = stderr_file { + stderr = Some(WritePipe::new(file)); + } else { + stderr = None + } + + Self { + stdin: ReadPipe::from(input), + stdout: WritePipe::new_in_memory(), + stderr + } + } + pub fn configure_wasi_ctx(&self, builder: WasiCtxBuilder) -> WasiCtxBuilder { + let builder = builder + .stdin(Box::new(self.stdin.clone())) + .stdout(Box::new(self.stdout.clone())); + + if let Some(pipe) = self.stderr.clone() { + builder.stderr(Box::new(pipe)) + } else { + builder.inherit_stderr() + } + } +} diff --git a/src/main.rs b/src/main.rs index ab79d40b..4ef68920 100644 --- a/src/main.rs +++ b/src/main.rs @@ -115,7 +115,7 @@ async fn main() -> std::io::Result<()> { ); } - let server = serve(&args.path, routes, &args.hostname, args.port) + let server = serve(&args.path, routes, &args.hostname, args.port, None) .await .map_err(|err| Error::new(ErrorKind::AddrInUse, err))?; From bd97b363acf26fe88123b1fece75c41de8144ac5 Mon Sep 17 00:00:00 2001 From: Angel M De Miguel Date: Mon, 8 May 2023 09:02:01 +0200 Subject: [PATCH 2/3] clean: minor code improvements --- crates/worker/src/stdio.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/worker/src/stdio.rs b/crates/worker/src/stdio.rs index bc451eb5..7b19404e 100644 --- a/crates/worker/src/stdio.rs +++ b/crates/worker/src/stdio.rs @@ -39,11 +39,14 @@ impl Stdio { stderr } } + pub fn configure_wasi_ctx(&self, builder: WasiCtxBuilder) -> WasiCtxBuilder { let builder = builder .stdin(Box::new(self.stdin.clone())) .stdout(Box::new(self.stdout.clone())); + // Set stderr if it was previously configured. If not, inherit + // it from the environment if let Some(pipe) = self.stderr.clone() { builder.stderr(Box::new(pipe)) } else { From 4a39586a6ac17a59db7fab3b85ec23260f6cea08 Mon Sep 17 00:00:00 2001 From: Angel M De Miguel Date: Mon, 8 May 2023 11:53:40 +0200 Subject: [PATCH 3/3] clean: fix format issues --- crates/server/src/handlers/worker.rs | 15 +++++++++------ crates/server/src/lib.rs | 7 ++----- crates/worker/src/lib.rs | 7 +++---- crates/worker/src/stdio.rs | 6 +++--- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/crates/server/src/handlers/worker.rs b/crates/server/src/handlers/worker.rs index f6559aac..e258bec6 100644 --- a/crates/server/src/handlers/worker.rs +++ b/crates/server/src/handlers/worker.rs @@ -8,7 +8,7 @@ use actix_web::{ web::{Bytes, Data}, HttpRequest, HttpResponse, }; -use std::{sync::RwLock, fs::File}; +use std::{fs::File, sync::RwLock}; use wws_router::Routes; use wws_worker::io::WasmOutput; @@ -69,11 +69,14 @@ pub async fn handle_worker(req: HttpRequest, body: Bytes) -> HttpResponse { None => None, }; - let (handler_result, handler_success) = match route.worker.run(&req, &body_str, store, vars, stderr_file.get_ref()) - { - Ok(output) => (output, true), - Err(_) => (WasmOutput::failed(), false), - }; + let (handler_result, handler_success) = + match route + .worker + .run(&req, &body_str, store, vars, stderr_file.get_ref()) + { + Ok(output) => (output, true), + Err(_) => (WasmOutput::failed(), false), + }; let mut builder = HttpResponse::build( StatusCode::from_u16(handler_result.status).unwrap_or(StatusCode::OK), diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 711504f4..f85591ce 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -33,7 +33,7 @@ pub async fn serve( base_routes: Routes, hostname: &str, port: u16, - stderr: Option<&Path> + stderr: Option<&Path>, ) -> Result { // Initializes the data connectors. For now, just KV let data = Data::new(RwLock::new(DataConnectors::default())); @@ -43,10 +43,7 @@ pub async fn serve( // Configure stderr if let Some(path) = stderr { - let file = OpenOptions::new() - .read(true) - .write(true) - .open(path)?; + let file = OpenOptions::new().read(true).write(true).open(path)?; stderr_file = Data::new(Some(file)); } else { diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 730c6a42..265e2c26 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -9,10 +9,10 @@ use actix_web::HttpRequest; use anyhow::{anyhow, Result}; use config::Config; use io::{WasmInput, WasmOutput}; -use stdio::Stdio; use std::fs::{self, File}; use std::path::PathBuf; use std::{collections::HashMap, path::Path}; +use stdio::Stdio; use wasmtime::{Engine, Linker, Module, Store}; use wasmtime_wasi::{Dir, WasiCtxBuilder}; use wws_config::Config as ProjectConfig; @@ -73,7 +73,7 @@ impl Worker { body: &str, kv: Option>, vars: &HashMap, - stderr: &Option + stderr: &Option, ) -> Result { let input = serde_json::to_string(&WasmInput::new(request, body, kv)).unwrap(); @@ -97,8 +97,7 @@ impl Worker { vars.iter().map(|(k, v)| (k.clone(), v.clone())).collect(); // Create the initial WASI context - let mut wasi_builder = WasiCtxBuilder::new() - .envs(&tuple_vars)?; + let mut wasi_builder = WasiCtxBuilder::new().envs(&tuple_vars)?; // Configure the stdio wasi_builder = stdio.configure_wasi_ctx(wasi_builder); diff --git a/crates/worker/src/stdio.rs b/crates/worker/src/stdio.rs index 7b19404e..49b94da7 100644 --- a/crates/worker/src/stdio.rs +++ b/crates/worker/src/stdio.rs @@ -1,4 +1,4 @@ -use std::{io::Cursor, fs::{File}}; +use std::{fs::File, io::Cursor}; use wasi_common::pipe::{ReadPipe, WritePipe}; use wasmtime_wasi::WasiCtxBuilder; @@ -19,7 +19,7 @@ pub struct Stdio { /// Defines the stdout to extract the data from the module pub stdout: WritePipe>>, /// Defines the stderr to expose logs from inside the module - pub stderr: Option> + pub stderr: Option>, } impl Stdio { @@ -36,7 +36,7 @@ impl Stdio { Self { stdin: ReadPipe::from(input), stdout: WritePipe::new_in_memory(), - stderr + stderr, } }