fix(runtime/js/workers): throw errors instead of using an op (#12249)

cli/tests/integration/worker_tests.rs

@@ -20,6 +20,20 @@ itest!(worker_nested_error {
   exit_code: 1,
 });
 
+itest!(worker_async_error {
+  args: "run -A --quiet --reload workers/worker_async_error.ts",
+  output: "workers/worker_async_error.ts.out",
+  http_server: true,
+  exit_code: 1,
+});
+
+itest!(worker_message_handler_error {
+  args: "run -A --quiet --reload workers/worker_message_handler_error.ts",
+  output: "workers/worker_message_handler_error.ts.out",
+  http_server: true,
+  exit_code: 1,
+});
+
 itest!(nonexistent_worker {
   args: "run --allow-read workers/nonexistent_worker.ts",
   output: "workers/nonexistent_worker.out",

cli/tests/testdata/worker_drop_handle_race.js.out

@@ -4,5 +4,5 @@ error: Uncaught (in worker "") Error
     at [WILDCARD]/workers/drop_handle_race.js:2:9
     at fire (deno:ext/timers/[WILDCARD])
     at handleTimerMacrotask (deno:ext/timers/[WILDCARD])
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl (deno:runtime/js/11_workers.js:[WILDCARD])

cli/tests/testdata/workers/async_error.ts

@@ -0,0 +1,4 @@
+// deno-lint-ignore require-await
+(async () => {
+  throw new Error("bar");
+})();

cli/tests/testdata/workers/message_handler_error.ts

@@ -0,0 +1,4 @@
+postMessage("ready");
+onmessage = () => {
+  throw new Error("bar");
+};

cli/tests/testdata/workers/nonexistent_worker.out

@@ -1,3 +1,3 @@
 [WILDCARD]error: Uncaught (in worker "") Cannot resolve module "file:///[WILDCARD]/workers/doesnt_exist.js".
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_blob_local.ts.out

@@ -1,4 +1,4 @@
 error: Uncaught (in worker "") Requires read access to "[WILDCARD]local_file.ts", run again with the --allow-read flag
     at blob:null/[WILDCARD]:1:0
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_blob_remote.ts.out

@@ -1,4 +1,4 @@
 error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
     at blob:null/[WILDCARD]:1:0
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_data_local.ts.out

@@ -1,4 +1,4 @@
 error: Uncaught (in worker "") Requires read access to "[WILDCARD]local_file.ts", run again with the --allow-read flag
     at data:application/javascript;base64,[WILDCARD]:1:0
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_data_remote.ts.out

@@ -1,4 +1,4 @@
 error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
     at data:application/javascript;base64,aW1wb3J0ICJodHRwczovL2V4YW1wbGUuY29tL3NvbWUvZmlsZS50cyI7:1:0
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_dynamic_remote.ts.out

@@ -2,5 +2,5 @@ error: Uncaught (in worker "") (in promise) TypeError: Requires net access to "e
 await import("https://example.com/some/file.ts");
 ^
     at async http://localhost:4545/workers/dynamic_remote.ts:2:1
-[WILDCARD]error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+[WILDCARD]error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/permissions_remote_remote.ts.out

@@ -1,4 +1,4 @@
 error: Uncaught (in worker "") Requires net access to "example.com", run again with the --allow-net flag
     at http://localhost:4545/workers/static_remote.ts:2:0
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/test.ts

@@ -34,11 +34,6 @@ Deno.test({
       tsWorker.postMessage("Hello World");
     };
 
-    jsWorker.onerror = (e: Event) => {
-      e.preventDefault();
-      jsWorker.postMessage("Hello World");
-    };
-
     jsWorker.postMessage("Hello World");
     await promise;
     tsWorker.terminate();

cli/tests/testdata/workers/test_worker.js

@@ -1,19 +1,8 @@
-let thrown = false;
-
 if (self.name !== "") {
   throw Error(`Bad worker name: ${self.name}, expected empty string.`);
 }
 
 onmessage = function (e) {
-  if (thrown === false) {
-    thrown = true;
-    throw new SyntaxError("[test error]");
-  }
-
   postMessage(e.data);
   close();
 };
-
-onerror = function () {
-  return false;
-};

cli/tests/testdata/workers/worker_async_error.ts

@@ -0,0 +1,5 @@
+const worker = new Worker(
+  new URL("async_error.ts", import.meta.url).href,
+  { type: "module", name: "foo" },
+);
+setTimeout(() => worker.terminate(), 30000);

cli/tests/testdata/workers/worker_async_error.ts.out

@@ -0,0 +1,7 @@
+error: Uncaught (in worker "foo") (in promise) Error: bar
+  throw new Error("bar");
+        ^
+    at [WILDCARD]/async_error.ts:[WILDCARD]
+    at [WILDCARD]/async_error.ts:[WILDCARD]
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
+    at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/worker_error.ts.out

@@ -1,5 +1,5 @@
 [WILDCARD]error: Uncaught (in worker "bar") Error: foo[WILDCARD]
     at foo ([WILDCARD])
     at [WILDCARD]
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/worker_message_handler_error.ts

@@ -0,0 +1,8 @@
+const worker = new Worker(
+  new URL("message_handler_error.ts", import.meta.url).href,
+  { type: "module", name: "foo" },
+);
+worker.onmessage = () => {
+  worker.postMessage("ready");
+};
+setTimeout(() => worker.terminate(), 30000);

cli/tests/testdata/workers/worker_message_handler_error.ts.out

@@ -0,0 +1,7 @@
+error: Uncaught (in worker "foo") (in promise) Error: bar
+  throw new Error("bar");
+        ^
+    at onmessage ([WILDCARD]/message_handler_error.ts:[WILDCARD])
+    at [WILDCARD]
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
+    at Worker.#pollControl ([WILDCARD])

cli/tests/testdata/workers/worker_nested_error.ts.out

@@ -1,5 +1,9 @@
 [WILDCARD]error: Uncaught (in worker "bar") Error: foo[WILDCARD]
-    at foo ([WILDCARD])
-    at [WILDCARD]
-error: Uncaught (in promise) Error: Unhandled error event reached main worker.
+  throw new Error("foo");
+        ^
+    at foo ([WILDCARD]/workers/error.ts:[WILDCARD])
+    at [WILDCARD]/workers/error.ts:[WILDCARD]
+error: Uncaught (in worker "baz") (in promise) Error: Unhandled error event in child worker.
+    at Worker.#pollControl ([WILDCARD])
+error: Uncaught (in promise) Error: Unhandled error event in child worker.
     at Worker.#pollControl ([WILDCARD])

runtime/js/11_workers.js

@@ -14,7 +14,6 @@
   } = window.__bootstrap.primordials;
   const webidl = window.__bootstrap.webidl;
   const { URL } = window.__bootstrap.url;
-  const { Window } = window.__bootstrap.globalInterfaces;
   const { getLocationHref } = window.__bootstrap.location;
   const { log, pathFromURL } = window.__bootstrap.util;
   const { defineEventHandler } = window.__bootstrap.webUtil;
@@ -265,14 +264,7 @@
           } /* falls through */
           case 2: { // Error
             if (!this.#handleError(data)) {
-              if (globalThis instanceof Window) {
-                throw new Error("Unhandled error event reached main worker.");
-              } else {
-                core.opSync(
-                  "op_worker_unhandled_error",
-                  data.message,
-                );
-              }
+              throw new Error("Unhandled error event in child worker.");
             }
             break;
           }

runtime/js/99_main.js

@@ -157,10 +157,7 @@ delete Object.prototype.__proto__;
 
         globalDispatchEvent(errorEvent);
         if (!errorEvent.defaultPrevented) {
-          core.opSync(
-            "op_worker_unhandled_error",
-            e.message,
-          );
+          throw e;
         }
       }
     }

runtime/ops/web_worker.rs

@@ -4,8 +4,6 @@ mod sync_fetch;
 
 use crate::web_worker::WebWorkerInternalHandle;
 use crate::web_worker::WebWorkerType;
-use crate::web_worker::WorkerControlEvent;
-use deno_core::error::generic_error;
 use deno_core::error::AnyError;
 use deno_core::op_async;
 use deno_core::op_sync;
@@ -25,11 +23,6 @@ pub fn init() -> Extension {
       ("op_worker_recv_message", op_async(op_worker_recv_message)),
       // Notify host that guest worker closes.
       ("op_worker_close", op_sync(op_worker_close)),
-      // Notify host that guest worker has unhandled error.
-      (
-        "op_worker_unhandled_error",
-        op_sync(op_worker_unhandled_error),
-      ),
       ("op_worker_get_type", op_sync(op_worker_get_type)),
       ("op_worker_sync_fetch", op_sync(op_worker_sync_fetch)),
     ])
@@ -70,23 +63,6 @@ fn op_worker_close(state: &mut OpState, _: (), _: ()) -> Result<(), AnyError> {
   Ok(())
 }
 
-/// A worker that encounters an uncaught error will pass this error
-/// to its parent worker using this op. The parent worker will use
-/// this same op to pass the error to its own parent (in case
-/// `e.preventDefault()` was not called in `worker.onerror`). This
-/// is done until the error reaches the root/ main worker.
-fn op_worker_unhandled_error(
-  state: &mut OpState,
-  message: String,
-  _: (),
-) -> Result<(), AnyError> {
-  let sender = state.borrow::<WebWorkerInternalHandle>().clone();
-  sender
-    .post_event(WorkerControlEvent::Error(generic_error(message)))
-    .expect("Failed to propagate error event to parent worker");
-  Ok(())
-}
-
 fn op_worker_get_type(
   state: &mut OpState,
   _: (),

runtime/web_worker.rs

@@ -525,15 +525,8 @@ impl WebWorker {
           return Poll::Ready(Ok(()));
         }
 
-        // In case of an error, pass to parent without terminating worker
         if let Err(e) = r {
-          print_worker_error(e.to_string(), &self.name);
-          let handle = self.internal_handle.clone();
-          handle
-            .post_event(WorkerControlEvent::Error(e))
-            .expect("Failed to post message to host");
-
-          return Poll::Pending;
+          return Poll::Ready(Err(e));
         }
 
         panic!(
@@ -593,6 +586,12 @@ pub fn run_web_worker(
       return Ok(());
     }
 
+    let result = if result.is_ok() {
+      worker.run_event_loop(true).await
+    } else {
+      result
+    };
+
     if let Err(e) = result {
       print_worker_error(e.to_string(), &name);
       internal_handle
@@ -603,7 +602,6 @@ pub fn run_web_worker(
       return Ok(());
     }
 
-    let result = worker.run_event_loop(true).await;
     debug!("Worker thread shuts down {}", &name);
     result
   };