From 118dd47ad0ced501b1ad8d9e1cc2a2f443f101e6 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 6 Sep 2022 12:18:23 +0100 Subject: [PATCH] fix(watch): ignore unload errors on drop (#15782) --- cli/tests/integration/watcher_tests.rs | 34 ++++++++++++++++++++++++++ cli/worker.rs | 18 ++++++++------ core/runtime.rs | 6 ++--- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index dee1987409..b69379427e 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -1038,6 +1038,40 @@ fn test_watch_module_graph_error_referrer() { check_alive_then_kill(child); } +// Regression test for https://github.com/denoland/deno/issues/15428. +#[test] +fn test_watch_unload_handler_error_on_drop() { + let t = TempDir::new(); + let file_to_watch = t.path().join("file_to_watch.js"); + write( + &file_to_watch, + r#" + addEventListener("unload", () => { + throw new Error("foo"); + }); + setTimeout(() => { + throw new Error("bar"); + }); + "#, + ) + .unwrap(); + let mut child = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("run") + .arg("--watch") + .arg(&file_to_watch) + .env("NO_COLOR", "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let (_, mut stderr_lines) = child_lines(&mut child); + wait_contains("Process started", &mut stderr_lines); + wait_contains("Uncaught Error: bar", &mut stderr_lines); + wait_contains("Process finished", &mut stderr_lines); + check_alive_then_kill(child); +} + #[test] fn run_watch_dynamic_imports() { let t = TempDir::new(); diff --git a/cli/worker.rs b/cli/worker.rs index 9eae6709b1..e766ec4aad 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -125,13 +125,18 @@ impl CliMainWorker { self.pending_unload = true; let result = loop { - let result = self.inner.worker.run_event_loop(false).await; - if !self + match self.inner.worker.run_event_loop(false).await { + Ok(()) => {} + Err(error) => break Err(error), + } + match self .inner .worker - .dispatch_beforeunload_event(&located_script_name!())? + .dispatch_beforeunload_event(&located_script_name!()) { - break result; + Ok(default_prevented) if default_prevented => {} // continue loop + Ok(_) => break Ok(()), + Err(error) => break Err(error), } }; self.pending_unload = false; @@ -152,11 +157,10 @@ impl CliMainWorker { impl Drop for FileWatcherModuleExecutor { fn drop(&mut self) { if self.pending_unload { - self + let _ = self .inner .worker - .dispatch_unload_event(&located_script_name!()) - .unwrap(); + .dispatch_unload_event(&located_script_name!()); } } } diff --git a/core/runtime.rs b/core/runtime.rs index 5b3b0ce501..42496b2712 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1167,11 +1167,11 @@ pub(crate) fn exception_to_err_result<'s, T>( // If termination is the result of a `op_dispatch_exception` call, we want // to use the exception that was passed to it rather than the exception that // was passed to this function. - let mut state = state_rc.borrow_mut(); + let state = state_rc.borrow(); exception = state .dispatched_exceptions - .pop_back() - .map(|exception| v8::Local::new(scope, exception)) + .back() + .map(|exception| v8::Local::new(scope, exception.clone())) .unwrap_or_else(|| { // Maybe make a new exception object. if was_terminating_execution && exception.is_null_or_undefined() {