fix: improve stability (#406)

* fix(sb_graph): don't propagate unnecessary panic

* fix(base): give the runtime an explicit chance to handle an interruption request by the supervisor

DESCRIPTION:

If we cancel the `termination_request_token` token too early, `DenoRuntime`
loses the chance to handle the interruption and may not provide a value for
`isolate_memory_usage_rx`, causing the task that called `create_supervisor` to
deadlock.

`DenoRuntime::run` is returned by canceling the `termination_request_token`
token, so the user will see the worker terminated as normal, but it will be
reported as `EventLoopCompleted`.

* stamp: apply `cargo fmt`

Author: nyannyacha

crates/base/src/rt_worker/worker_ctx.rs

@@ -376,6 +376,8 @@ pub fn create_supervisor(
                 use deno_core::futures::channel::mpsc;
                 use deno_core::serde_json::Value;
 
+                let termination_request_token = termination_request_token.clone();
+
                 base_rt::SUPERVISOR_RT
                     .spawn_blocking(move || {
                         let wait_inspector_disconnect_fut = async move {
@@ -452,8 +454,6 @@ pub fn create_supervisor(
                     })
                     .await
                     .unwrap();
-            } else {
-                termination_request_token.cancel();
             }
 
             // NOTE: If we issue a hard CPU time limit, It's OK because it is
@@ -494,6 +494,11 @@ pub fn create_supervisor(
                 }
             };
 
+            if !termination_request_token.is_cancelled() {
+                termination_request_token.cancel();
+                waker.wake();
+            }
+
             // send termination reason
             let termination_event = WorkerEvents::Shutdown(ShutdownEvent {
                 reason,

crates/base/tests/integration_tests.rs

@@ -4,6 +4,7 @@ mod integration_test_helper;
 use http_v02 as http;
 use hyper_v014 as hyper;
 use reqwest_v011 as reqwest;
+use sb_graph::EszipPayloadKind;
 
 use std::{
     borrow::Cow,
@@ -883,6 +884,62 @@ async fn test_worker_boot_invalid_imports() {
         .starts_with("worker boot error"));
 }
 
+#[tokio::test]
+#[serial]
+async fn test_worker_boot_with_0_byte_eszip() {
+    let opts = WorkerContextInitOpts {
+        service_path: "./test_cases/meow".into(),
+        no_module_cache: false,
+        import_map_path: None,
+        env_vars: HashMap::new(),
+        events_rx: None,
+        timing: None,
+        maybe_eszip: Some(EszipPayloadKind::VecKind(vec![])),
+        maybe_entrypoint: Some("file:///src/index.ts".to_string()),
+        maybe_decorator: None,
+        maybe_module_code: None,
+        conf: WorkerRuntimeOpts::UserWorker(test_user_runtime_opts()),
+        static_patterns: vec![],
+        maybe_jsx_import_source_config: None,
+    };
+
+    let result = create_test_user_worker(opts).await;
+
+    assert!(result.is_err());
+    assert!(result
+        .unwrap_err()
+        .to_string()
+        .starts_with("worker boot error: unexpected end of file"));
+}
+
+#[tokio::test]
+#[serial]
+async fn test_worker_boot_with_invalid_entrypoint() {
+    let opts = WorkerContextInitOpts {
+        service_path: "./test_cases/meow".into(),
+        no_module_cache: false,
+        import_map_path: None,
+        env_vars: HashMap::new(),
+        events_rx: None,
+        timing: None,
+        maybe_eszip: None,
+        maybe_entrypoint: Some("file:///meow/mmmmeeeow.ts".to_string()),
+        maybe_decorator: None,
+        maybe_module_code: None,
+        conf: WorkerRuntimeOpts::UserWorker(test_user_runtime_opts()),
+        static_patterns: vec![],
+        maybe_jsx_import_source_config: None,
+    };
+
+    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"));
+}
+
 #[tokio::test]
 #[serial]
 async fn req_failure_case_timeout() {

crates/sb_graph/eszip_migrate.rs

@@ -422,7 +422,9 @@ mod test {
     }
 
     async fn test_vfs_npm_registry_migration_1_45_x(buf: Vec<u8>) {
-        let eszip = payload_to_eszip(EszipPayloadKind::VecKind(buf)).await;
+        let eszip = payload_to_eszip(EszipPayloadKind::VecKind(buf))
+            .await
+            .unwrap();
         let migrated = try_migrate_if_needed(eszip).await.unwrap();
 
         let vfs_data = migrated

crates/sb_graph/graph_util.rs

@@ -412,11 +412,11 @@ pub async fn create_graph(
 
         specifier
     } else {
-        let binding = std::fs::canonicalize(&file).unwrap();
-        let specifier = binding.to_str().unwrap();
+        let binding = std::fs::canonicalize(&file).context("failed to read path")?;
+        let specifier = binding.to_str().context("failed to convert path to str")?;
         let format_specifier = format!("file:///{}", specifier);
 
-        ModuleSpecifier::parse(&format_specifier).unwrap()
+        ModuleSpecifier::parse(&format_specifier).context("failed to parse specifier")?
     };
 
     let builder = ModuleGraphBuilder::new(emitter_factory, false);

crates/sb_graph/lib.rs

@@ -597,9 +597,11 @@ impl EszipDataSection {
     }
 }
 
-pub async fn payload_to_eszip(eszip_payload_kind: EszipPayloadKind) -> LazyLoadableEszip {
+pub async fn payload_to_eszip(
+    eszip_payload_kind: EszipPayloadKind,
+) -> Result<LazyLoadableEszip, anyhow::Error> {
     match eszip_payload_kind {
-        EszipPayloadKind::Eszip(eszip) => LazyLoadableEszip::new(eszip, None),
+        EszipPayloadKind::Eszip(eszip) => Ok(LazyLoadableEszip::new(eszip, None)),
         _ => {
             let bytes = match eszip_payload_kind {
                 EszipPayloadKind::JsBufferKind(js_buffer) => Vec::from(&*js_buffer),
@@ -610,7 +612,7 @@ pub async fn payload_to_eszip(eszip_payload_kind: EszipPayloadKind) -> LazyLoada
             let mut io = AllowStdIo::new(Cursor::new(bytes));
             let mut bufreader = BufReader::new(&mut io);
 
-            let eszip = eszip_parse::parse_v2_header(&mut bufreader).await.unwrap();
+            let eszip = eszip_parse::parse_v2_header(&mut bufreader).await?;
 
             let initial_offset = bufreader.stream_position().await.unwrap();
             let data_section = EszipDataSection::new(
@@ -620,7 +622,7 @@ pub async fn payload_to_eszip(eszip_payload_kind: EszipPayloadKind) -> LazyLoada
                 eszip.options,
             );
 
-            LazyLoadableEszip::new(eszip, Some(Arc::new(data_section)))
+            Ok(LazyLoadableEszip::new(eszip, Some(Arc::new(data_section))))
         }
     }
 }
@@ -858,14 +860,21 @@ async fn extract_modules(
 
 pub async fn extract_eszip(payload: ExtractEszipPayload) -> bool {
     let output_folder = payload.folder;
-    let mut eszip =
-        match eszip_migrate::try_migrate_if_needed(payload_to_eszip(payload.data).await).await {
-            Ok(v) => v,
-            Err(_old) => {
-                error!("eszip migration failed (give up extract job)");
-                return false;
-            }
-        };
+    let eszip = match payload_to_eszip(payload.data).await {
+        Ok(v) => v,
+        Err(err) => {
+            error!("{err:?}");
+            return false;
+        }
+    };
+
+    let mut eszip = match eszip_migrate::try_migrate_if_needed(eszip).await {
+        Ok(v) => v,
+        Err(_old) => {
+            error!("eszip migration failed (give up extract job)");
+            return false;
+        }
+    };
 
     eszip.ensure_read_all().await.unwrap();
 

crates/sb_module_loader/standalone/mod.rs

@@ -222,16 +222,15 @@ pub async fn create_module_loader_for_standalone_from_eszip_kind<P>(
 where
     P: AsRef<Path>,
 {
-    let eszip = match eszip_migrate::try_migrate_if_needed(
-        payload_to_eszip(eszip_payload_kind).await,
-    )
-    .await
-    {
-        Ok(v) => v,
-        Err(_old) => {
-            bail!("eszip migration failed");
-        }
-    };
+    let eszip =
+        match eszip_migrate::try_migrate_if_needed(payload_to_eszip(eszip_payload_kind).await?)
+            .await
+        {
+            Ok(v) => v,
+            Err(_old) => {
+                bail!("eszip migration failed");
+            }
+        };
 
     let maybe_import_map = 'scope: {
         if maybe_import_map.is_some() {