Handle uncaught worker errors without panicking (denoland#3019)

gcollazo · Sep 25, 2019 · 3d2d0ee · 3d2d0ee
1 parent 112ce0d
commit 3d2d0ee
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 15 deletions.
diff --git a/cli/deno_error.rs b/cli/deno_error.rs
@@ -22,6 +22,17 @@ pub struct DenoError {
  msg: String,
 }
 
+pub fn print_err_and_exit(err: ErrBox) {
+ eprintln!("{}", err.to_string());
+ std::process::exit(1);
+}
+
+pub fn js_check(r: Result<(), ErrBox>) {
+ if let Err(err) = r {
+ print_err_and_exit(err);
+ }
+}
+
 impl DenoError {
  pub fn new(kind: ErrorKind, msg: String) -> Self {
  Self { kind, msg }

diff --git a/cli/lib.rs b/cli/lib.rs
@@ -51,6 +51,8 @@ mod tokio_write;
 pub mod version;
 pub mod worker;
 
+use crate::deno_error::js_check;
+use crate::deno_error::print_err_and_exit;
 use crate::progress::Progress;
 use crate::state::ThreadSafeState;
 use crate::worker::Worker;
@@ -90,17 +92,6 @@ impl log::Log for Logger {
  fn flush(&self) {}
 }
 
-fn print_err_and_exit(err: ErrBox) {
- eprintln!("{}", err.to_string());
- std::process::exit(1);
-}
-
-fn js_check(r: Result<(), ErrBox>) {
- if let Err(err) = r {
- print_err_and_exit(err);
- }
-}
-
 fn create_worker_and_state(
  flags: DenoFlags,
  argv: Vec<String>,
@@ -118,7 +109,7 @@ fn create_worker_and_state(
  });
  // TODO(kevinkassimo): maybe make include_deno_namespace also configurable?
  let state = ThreadSafeState::new(flags, argv, progress, true)
- .map_err(print_err_and_exit)
+ .map_err(deno_error::print_err_and_exit)
  .unwrap();
  let worker = Worker::new(
  "main".to_string(),

diff --git a/cli/ops/workers.rs b/cli/ops/workers.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
 use super::dispatch_json::{Deserialize, JsonOp, Value};
+use crate::deno_error::js_check;
 use crate::deno_error::DenoError;
 use crate::deno_error::ErrorKind;
 use crate::resources;
@@ -124,8 +125,8 @@ pub fn op_create_worker(
 
  let mut worker =
  Worker::new(name, startup_data::deno_isolate_init(), child_state);
- worker.execute(&deno_main_call).unwrap();
- worker.execute("workerMain()").unwrap();
+ js_check(worker.execute(&deno_main_call));
+ js_check(worker.execute("workerMain()"));
 
  let exec_cb = move |worker: Worker| {
  let mut workers_tl = parent_state.workers.lock().unwrap();
@@ -135,7 +136,7 @@ pub fn op_create_worker(
 
  // Has provided source code, execute immediately.
  if has_source_code {
- worker.execute(&source_code).unwrap();
+ js_check(worker.execute(&source_code));
  return Ok(JsonOp::Sync(exec_cb(worker)));
  }
 

diff --git a/cli/tests/error_worker_dynamic.ts b/cli/tests/error_worker_dynamic.ts
@@ -0,0 +1,3 @@
+const b = new Blob(['throw new Error("hello");']);
+const blobURL = URL.createObjectURL(b);
+new Worker(blobURL);
diff --git a/cli/tests/error_worker_dynamic.ts.out b/cli/tests/error_worker_dynamic.ts.out
@@ -0,0 +1,3 @@
+[WILDCARD]error: Uncaught Error: hello
+[WILDCARD]__anonymous__:1:7
+ at [WILDCARD]__anonymous__:1:7
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
@@ -474,6 +474,13 @@ itest!(error_type_definitions {
  output: "error_type_definitions.ts.out",
 });
 
+itest!(error_worker_dynamic {
+ args: "run --reload error_worker_dynamic.ts",
+ check_stderr: true,
+ exit_code: 1,
+ output: "error_worker_dynamic.ts.out",
+});
+
 itest!(exit_error42 {
  exit_code: 42,
  args: "run --reload exit_error42.ts",