fix: render detailed error message for failed to create a graph (#418)

* chore(base_rt): add a dependency

* chore: update `Cargo.lock`

* fix: render detailed error message for failed to create a graph

* chore: add an integration test for graph errors

Author: nyannyacha

Cargo.lock

@@ -13,9 +13,9 @@ use tokio::sync::oneshot::Receiver;
 
 impl WorkerHandler for Worker {
     fn handle_error(&self, error: Error) -> Result<WorkerEvents, Error> {
-        log::error!("{}", error);
+        log::error!("{}", format!("{error:#}"));
         Ok(WorkerEvents::BootFailure(BootFailureEvent {
-            msg: error.to_string(),
+            msg: format!("{error:#}"),
         }))
     }
 

crates/base/src/rt_worker/implementation/default_handler.rs

@@ -4,8 +4,9 @@ use crate::rt_worker::supervisor;
 use crate::rt_worker::utils::{get_event_metadata, parse_worker_conf};
 use crate::rt_worker::worker_ctx::create_supervisor;
 use crate::utils::send_event_if_event_worker_available;
-use anyhow::{anyhow, Error};
+use anyhow::Error;
 use base_mem_check::MemCheckState;
+use base_rt::error::CloneableError;
 use event_worker::events::{
     EventLoopCompletedEvent, EventMetadata, ShutdownEvent, ShutdownReason, UncaughtExceptionEvent,
     WorkerEventWithMetadata, WorkerEvents, WorkerMemoryUsed,
@@ -313,10 +314,10 @@ impl Worker {
                     Err(err) => {
                         drop(permit);
 
-                        let _ = booter_signal
-                            .send(Err(anyhow!("worker boot error: {err}")));
+                        let err = CloneableError::from(err.context("worker boot error"));
+                        let _ = booter_signal.send(Err(err.clone().into()));
 
-                        method_cloner.handle_error(err)
+                        method_cloner.handle_error(err.into())
                     }
                 };
 

crates/base/src/rt_worker/worker.rs

@@ -640,19 +640,7 @@ pub async fn create_worker<Opt: Into<CreateWorkerArgs>>(
         });
 
         // wait for worker to be successfully booted
-        let worker_boot_result = worker_boot_result_rx.await?;
-
-        match worker_boot_result {
-            Err(err) => {
-                worker_req_handle.abort();
-
-                if let Some(token) = maybe_termination_token.as_ref() {
-                    token.outbound.cancel();
-                }
-
-                bail!(err)
-            }
-
+        match worker_boot_result_rx.await? {
             Ok(metric) => {
                 let elapsed = worker_struct_ref
                     .worker_boot_start_time
@@ -673,6 +661,15 @@ pub async fn create_worker<Opt: Into<CreateWorkerArgs>>(
                     exit,
                 })
             }
+            Err(err) => {
+                worker_req_handle.abort();
+
+                if let Some(token) = maybe_termination_token.as_ref() {
+                    token.outbound.cancel();
+                }
+
+                Err(err)
+            }
         }
     } else {
         bail!("Unknown")

crates/base/src/rt_worker/worker_ctx.rs

@@ -452,11 +452,10 @@ impl WorkerPool {
 
                     status.demand.fetch_add(1, Ordering::Release);
                 }
-                Err(e) => {
-                    if tx.send(Err(e)).is_err() {
+                Err(err) => {
+                    error!("{err:#}");
+                    if tx.send(Err(err)).is_err() {
                         error!("main worker receiver dropped")
-                    } else {
-                        error!("An error has occured")
                     }
                 }
             }

crates/base/src/rt_worker/worker_pool.rs

@@ -0,0 +1,7 @@
+// Relative import path "oak" not prefixed with / or ./ or ../
+import "oak";
+export default {
+    fetch() {
+        return new Response("meow");
+    }
+}

crates/base/test_cases/graph-error-1/index.ts

@@ -0,0 +1,7 @@
+// Module not found "file:///.../meow.ts
+import "../meow.ts";
+export default {
+    fetch() {
+        return new Response("meow");
+    }
+}

crates/base/test_cases/graph-error-2/index.ts

@@ -906,9 +906,7 @@ async fn test_worker_boot_with_0_byte_eszip() {
     let result = create_test_user_worker(opts).await;
 
     assert!(result.is_err());
-    assert!(result
-        .unwrap_err()
-        .to_string()
+    assert!(format!("{:#}", result.unwrap_err())
         .starts_with("worker boot error: unexpected end of file"));
 }
 
@@ -934,10 +932,9 @@ async fn test_worker_boot_with_invalid_entrypoint() {
     let result = create_test_user_worker(opts).await;
 
     assert!(result.is_err());
-    assert!(result
-        .unwrap_err()
-        .to_string()
-        .starts_with("worker boot error: failed to read path"));
+    assert!(
+        format!("{:#}", result.unwrap_err()).starts_with("worker boot error: failed to read path")
+    );
 }
 
 #[tokio::test]
@@ -1552,11 +1549,6 @@ async fn test_decorators(ty: Option<DecoratorType>) {
     );
 }
 
-#[derive(Deserialize)]
-struct ErrorResponsePayload {
-    msg: String,
-}
-
 #[tokio::test]
 #[serial]
 async fn test_decorator_should_be_syntax_error() {
@@ -2348,6 +2340,71 @@ async fn test_issue_420() {
     );
 }
 
+#[tokio::test]
+#[serial]
+async fn test_should_render_detailed_failed_to_create_graph_error() {
+    {
+        integration_test!(
+            "./test_cases/main",
+            NON_SECURE_PORT,
+            "graph-error-1",
+            None,
+            None,
+            None,
+            None,
+            (|resp| async {
+                let (payload, status) = ErrorResponsePayload::assert_error_response(resp).await;
+
+                assert_eq!(status, 500);
+                assert!(payload.msg.starts_with(
+                    "InvalidWorkerCreation: worker boot error: failed to create the graph: \
+                    Relative import path \"oak\" not prefixed with"
+                ));
+            }),
+            TerminationToken::new()
+        );
+    }
+
+    {
+        integration_test!(
+            "./test_cases/main",
+            NON_SECURE_PORT,
+            "graph-error-2",
+            None,
+            None,
+            None,
+            None,
+            (|resp| async {
+                let (payload, status) = ErrorResponsePayload::assert_error_response(resp).await;
+
+                assert_eq!(status, 500);
+                assert!(payload.msg.starts_with(
+                    "InvalidWorkerCreation: worker boot error: failed to create the graph: \
+                    Module not found \"file://"
+                ));
+            }),
+            TerminationToken::new()
+        );
+    }
+}
+
+#[derive(Deserialize)]
+struct ErrorResponsePayload {
+    msg: String,
+}
+
+impl ErrorResponsePayload {
+    async fn assert_error_response(resp: Result<Response, reqwest::Error>) -> (Self, u16) {
+        let res = resp.unwrap();
+        let status = res.status().as_u16();
+        let res = res.json::<Self>().await;
+
+        assert!(res.is_ok());
+
+        (res.unwrap(), status)
+    }
+}
+
 trait AsyncReadWrite: AsyncRead + AsyncWrite + Send + Unpin {}
 
 impl<T> AsyncReadWrite for T where T: AsyncRead + AsyncWrite + Send + Unpin {}

crates/base/tests/integration_tests.rs

@@ -4,6 +4,8 @@ version = "0.1.0"
 edition = "2021"
 
 [dependencies]
+deno_core.workspace = true
+
 tokio.workspace = true
 once_cell.workspace = true
 tokio-util = { workspace = true, features = ["rt"] }

crates/base_rt/Cargo.toml

@@ -0,0 +1,34 @@
+use std::sync::Arc;
+
+use deno_core::anyhow;
+
+#[derive(Clone)]
+pub struct CloneableError {
+    inner: Arc<anyhow::Error>,
+}
+
+impl From<anyhow::Error> for CloneableError {
+    fn from(value: anyhow::Error) -> Self {
+        Self {
+            inner: Arc::new(value),
+        }
+    }
+}
+
+impl std::fmt::Display for CloneableError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.inner.fmt(f)
+    }
+}
+
+impl std::fmt::Debug for CloneableError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.inner.fmt(f)
+    }
+}
+
+impl std::error::Error for CloneableError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        self.inner.source()
+    }
+}