fix(ext/node): dispatch beforeExit/exit events irrespective of listen…

…ers (denoland#23382)

Closes denoland#23342
Closes denoland#21757

cli/tools/bench/mod.rs

@@ -269,7 +269,9 @@ async fn bench_specifier_inner(
   // Ignore `defaultPrevented` of the `beforeunload` event. We don't allow the
   // event loop to continue beyond what's needed to await results.
   worker.dispatch_beforeunload_event()?;
+  worker.dispatch_process_beforeexit_event()?;
   worker.dispatch_unload_event()?;
+  worker.dispatch_process_exit_event()?;
 
   // Ensure the worker has settled so we can catch any remaining unhandled rejections. We don't
   // want to wait forever here.

cli/tools/coverage/mod.rs

@@ -71,6 +71,7 @@ impl crate::worker::CoverageCollector for CoverageCollector {
       // Filter out internal JS files from being included in coverage reports
       if script_coverage.url.starts_with("ext:")
         || script_coverage.url.starts_with("[ext:")
+        || script_coverage.url.starts_with("node:")
       {
         continue;
       }

cli/worker.rs

@@ -212,12 +212,17 @@ impl CliMainWorker {
           .await?;
       }
 
-      if !self.worker.dispatch_beforeunload_event()? {
-        break;
+      let web_continue = self.worker.dispatch_beforeunload_event()?;
+      if !web_continue {
+        let node_continue = self.worker.dispatch_process_beforeexit_event()?;
+        if !node_continue {
+          break;
+        }
       }
     }
 
     self.worker.dispatch_unload_event()?;
+    self.worker.dispatch_process_exit_event()?;
 
     if let Some(coverage_collector) = maybe_coverage_collector.as_mut() {
       self
@@ -272,17 +277,21 @@ impl CliMainWorker {
             Ok(()) => {}
             Err(error) => break Err(error),
           }
-          match self.inner.worker.dispatch_beforeunload_event() {
-            Ok(default_prevented) if default_prevented => {} // continue loop
-            Ok(_) => break Ok(()),
-            Err(error) => break Err(error),
+          let web_continue = self.inner.worker.dispatch_beforeunload_event()?;
+          if !web_continue {
+            let node_continue =
+              self.inner.worker.dispatch_process_beforeexit_event()?;
+            if !node_continue {
+              break Ok(());
+            }
           }
         };
         self.pending_unload = false;
 
         result?;
 
         self.inner.worker.dispatch_unload_event()?;
+        self.inner.worker.dispatch_process_exit_event()?;
 
         Ok(())
       }

ext/node/polyfills/process.ts

@@ -763,15 +763,13 @@ function processOnError(event: ErrorEvent) {
   uncaughtExceptionHandler(event.error, "uncaughtException");
 }
 
-function processOnBeforeUnload(event: Event) {
+function dispatchProcessBeforeExitEvent() {
   process.emit("beforeExit", process.exitCode || 0);
   processTicksAndRejections();
-  if (core.eventLoopHasMoreWork()) {
-    event.preventDefault();
-  }
+  return core.eventLoopHasMoreWork();
 }
 
-function processOnUnload() {
+function dispatchProcessExitEvent() {
   if (!process._exiting) {
     process._exiting = true;
     process.emit("exit", process.exitCode || 0);
@@ -817,16 +815,6 @@ function synchronizeListeners() {
   } else {
     globalThis.removeEventListener("error", processOnError);
   }
-  if (beforeExitListenerCount > 0) {
-    globalThis.addEventListener("beforeunload", processOnBeforeUnload);
-  } else {
-    globalThis.removeEventListener("beforeunload", processOnBeforeUnload);
-  }
-  if (exitListenerCount > 0) {
-    globalThis.addEventListener("unload", processOnUnload);
-  } else {
-    globalThis.removeEventListener("unload", processOnUnload);
-  }
 }
 
 // Overwrites the 1st and 2nd items with getters.
@@ -841,6 +829,8 @@ Object.defineProperty(argv, "1", {
   },
 });
 
+internals.dispatchProcessBeforeExitEvent = dispatchProcessBeforeExitEvent;
+internals.dispatchProcessExitEvent = dispatchProcessExitEvent;
 // Should be called only once, in `runtime/js/99_main.js` when the runtime is
 // bootstrapped.
 internals.__bootstrapNodeProcess = function (

runtime/js/99_main.js

@@ -1005,13 +1005,19 @@ function bootstrapWorkerRuntime(
 
 const nodeBootstrap = globalThis.nodeBootstrap;
 delete globalThis.nodeBootstrap;
+const dispatchProcessExitEvent = internals.dispatchProcessExitEvent;
+delete internals.dispatchProcessExitEvent;
+const dispatchProcessBeforeExitEvent = internals.dispatchProcessBeforeExitEvent;
+delete internals.dispatchProcessBeforeExitEvent;
 
 globalThis.bootstrap = {
   mainRuntime: bootstrapMainRuntime,
   workerRuntime: bootstrapWorkerRuntime,
   dispatchLoadEvent,
   dispatchUnloadEvent,
   dispatchBeforeUnloadEvent,
+  dispatchProcessExitEvent,
+  dispatchProcessBeforeExitEvent,
 };
 
 event.setEventTargetData(globalThis);

runtime/worker.rs

@@ -117,6 +117,8 @@ pub struct MainWorker {
   dispatch_load_event_fn_global: v8::Global<v8::Function>,
   dispatch_beforeunload_event_fn_global: v8::Global<v8::Function>,
   dispatch_unload_event_fn_global: v8::Global<v8::Function>,
+  dispatch_process_beforeexit_event_fn_global: v8::Global<v8::Function>,
+  dispatch_process_exit_event_fn_global: v8::Global<v8::Function>,
 }
 
 pub struct WorkerOptions {
@@ -530,6 +532,8 @@ impl MainWorker {
       dispatch_load_event_fn_global,
       dispatch_beforeunload_event_fn_global,
       dispatch_unload_event_fn_global,
+      dispatch_process_beforeexit_event_fn_global,
+      dispatch_process_exit_event_fn_global,
     ) = {
       let context = js_runtime.main_context();
       let scope = &mut js_runtime.handle_scope();
@@ -576,11 +580,39 @@ impl MainWorker {
         .unwrap();
       let dispatch_unload_event_fn =
         v8::Local::<v8::Function>::try_from(dispatch_unload_event_fn).unwrap();
+      let dispatch_process_beforeexit_event =
+        v8::String::new_external_onebyte_static(
+          scope,
+          b"dispatchProcessBeforeExitEvent",
+        )
+        .unwrap();
+      let dispatch_process_beforeexit_event_fn = bootstrap_ns
+        .get(scope, dispatch_process_beforeexit_event.into())
+        .unwrap();
+      let dispatch_process_beforeexit_event_fn =
+        v8::Local::<v8::Function>::try_from(
+          dispatch_process_beforeexit_event_fn,
+        )
+        .unwrap();
+      let dispatch_process_exit_event =
+        v8::String::new_external_onebyte_static(
+          scope,
+          b"dispatchProcessExitEvent",
+        )
+        .unwrap();
+      let dispatch_process_exit_event_fn = bootstrap_ns
+        .get(scope, dispatch_process_exit_event.into())
+        .unwrap();
+      let dispatch_process_exit_event_fn =
+        v8::Local::<v8::Function>::try_from(dispatch_process_exit_event_fn)
+          .unwrap();
       (
         v8::Global::new(scope, bootstrap_fn),
         v8::Global::new(scope, dispatch_load_event_fn),
         v8::Global::new(scope, dispatch_beforeunload_event_fn),
         v8::Global::new(scope, dispatch_unload_event_fn),
+        v8::Global::new(scope, dispatch_process_beforeexit_event_fn),
+        v8::Global::new(scope, dispatch_process_exit_event_fn),
       )
     };
 
@@ -594,6 +626,8 @@ impl MainWorker {
       dispatch_load_event_fn_global,
       dispatch_beforeunload_event_fn_global,
       dispatch_unload_event_fn_global,
+      dispatch_process_beforeexit_event_fn_global,
+      dispatch_process_exit_event_fn_global,
     }
   }
 
@@ -782,6 +816,21 @@ impl MainWorker {
     Ok(())
   }
 
+  /// Dispatches process.emit("exit") event for node compat.
+  pub fn dispatch_process_exit_event(&mut self) -> Result<(), AnyError> {
+    let scope = &mut self.js_runtime.handle_scope();
+    let tc_scope = &mut v8::TryCatch::new(scope);
+    let dispatch_process_exit_event_fn =
+      v8::Local::new(tc_scope, &self.dispatch_process_exit_event_fn_global);
+    let undefined = v8::undefined(tc_scope);
+    dispatch_process_exit_event_fn.call(tc_scope, undefined.into(), &[]);
+    if let Some(exception) = tc_scope.exception() {
+      let error = JsError::from_v8_exception(tc_scope, exception);
+      return Err(error.into());
+    }
+    Ok(())
+  }
+
   /// Dispatches "beforeunload" event to the JavaScript runtime. Returns a boolean
   /// indicating if the event was prevented and thus event loop should continue
   /// running.
@@ -800,4 +849,28 @@ impl MainWorker {
     let ret_val = ret_val.unwrap();
     Ok(ret_val.is_false())
   }
+
+  /// Dispatches process.emit("beforeExit") event for node compat.
+  pub fn dispatch_process_beforeexit_event(
+    &mut self,
+  ) -> Result<bool, AnyError> {
+    let scope = &mut self.js_runtime.handle_scope();
+    let tc_scope = &mut v8::TryCatch::new(scope);
+    let dispatch_process_beforeexit_event_fn = v8::Local::new(
+      tc_scope,
+      &self.dispatch_process_beforeexit_event_fn_global,
+    );
+    let undefined = v8::undefined(tc_scope);
+    let ret_val = dispatch_process_beforeexit_event_fn.call(
+      tc_scope,
+      undefined.into(),
+      &[],
+    );
+    if let Some(exception) = tc_scope.exception() {
+      let error = JsError::from_v8_exception(tc_scope, exception);
+      return Err(error.into());
+    }
+    let ret_val = ret_val.unwrap();
+    Ok(ret_val.is_true())
+  }
 }

tests/integration/node_compat_tests.rs

@@ -21,3 +21,17 @@ itest!(node_test_module_no_sanitizers {
   // exit_code: 123,
   http_server: true,
 });
+
+itest!(
+  node_process_beforeexit_exit_events_emitted_without_listeners {
+    args: "run node/process_beforeexit_exit_events.ts",
+    output: "node/process_beforeexit_exit_events.out",
+    exit_code: 0,
+  }
+);
+
+itest!(web_node_events_dispatched_in_correct_order {
+  args: "run node/events_order.ts",
+  output: "node/events_order.out",
+  exit_code: 0,
+});

tests/testdata/node/events_order.out

@@ -0,0 +1,12 @@
+beforeunload emitted from addEventListener
+beforeunload emitted from addEventListener
+beforeunload emitted from addEventListener
+beforeExit emitted from process.on
+more work done! 1
+beforeunload emitted from addEventListener
+beforeExit emitted from process.on
+more work done! 2
+beforeunload emitted from addEventListener
+beforeExit emitted from process.on
+unload emitted from addEventListener
+exit emitted from process.on

tests/testdata/node/events_order.ts

@@ -0,0 +1,25 @@
+import process from "node:process";
+
+let count = 0;
+process.on("beforeExit", () => {
+  if (count === 0 || count === 1) {
+    setTimeout(() => console.log("more work done!", count), 10);
+  }
+  count++;
+  console.log("beforeExit emitted from process.on");
+});
+process.on("exit", () => console.log("exit emitted from process.on"));
+
+let countWeb = 0;
+addEventListener("beforeunload", (event) => {
+  if (countWeb == 0 || countWeb == 1) {
+    event.preventDefault();
+  }
+  countWeb++;
+  console.log("beforeunload emitted from addEventListener");
+});
+
+addEventListener(
+  "unload",
+  () => console.log("unload emitted from addEventListener"),
+);

tests/testdata/node/process_beforeexit_exit_events.out

@@ -0,0 +1,2 @@
+beforeExit emitted from processEmit
+exit emitted from processEmit

tests/testdata/node/process_beforeexit_exit_events.ts

@@ -0,0 +1,9 @@
+import process from "node:process";
+
+const originalEmit = process.emit;
+process.emit = function (event, ...args) {
+  if (event === "exit" || event === "beforeExit") {
+    console.log(`${event} emitted from processEmit`);
+  }
+  return originalEmit.call(this, event, ...args);
+};