From 9ef0b18eb0b4c337ccfc8d0add36bec6b657262f Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Thu, 19 Dec 2019 21:04:14 -0800 Subject: [PATCH] repl: do not crash on async op reject (#3527) --- cli/lib.rs | 5 +++++ cli/worker.rs | 8 ++++++++ core/isolate.rs | 37 +++++++++++++++++++++++++++++-------- core/libdeno.rs | 1 + core/libdeno/api.cc | 6 ++++++ core/libdeno/deno.h | 6 ++++++ deno_typescript/lib.rs | 2 +- 7 files changed, 56 insertions(+), 9 deletions(-) diff --git a/cli/lib.rs b/cli/lib.rs index df4cbbdf0e..b191d5e876 100644 --- a/cli/lib.rs +++ b/cli/lib.rs @@ -349,6 +349,11 @@ fn bundle_command(flags: DenoFlags) { fn run_repl(flags: DenoFlags) { let (mut worker, _state) = create_worker_and_state(flags); + // Make repl continue to function under uncaught async errors. + worker.set_error_handler(Box::new(|err| { + eprintln!("{}", err.to_string()); + Ok(()) + })); // Setup runtime. js_check(worker.execute("denoMain()")); let main_future = async move { diff --git a/cli/worker.rs b/cli/worker.rs index 814e7f4402..e35458d39e 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -96,6 +96,14 @@ impl Worker { } } + pub fn set_error_handler( + &mut self, + handler: Box Result<(), ErrBox>>, + ) { + let mut i = self.isolate.lock().unwrap(); + i.set_error_handler(handler); + } + /// Same as execute2() but the filename defaults to "$CWD/__anonymous__". pub fn execute(&mut self, js_source: &str) -> Result<(), ErrBox> { let path = env::current_dir().unwrap().join("__anonymous__"); diff --git a/core/isolate.rs b/core/isolate.rs index 12ddb529a6..d2822e2b9a 100644 --- a/core/isolate.rs +++ b/core/isolate.rs @@ -31,6 +31,7 @@ use futures::stream::TryStreamExt; use futures::task::AtomicWaker; use libc::c_char; use libc::c_void; +use libc::strdup; use std::ffi::CStr; use std::ffi::CString; use std::fmt; @@ -158,6 +159,7 @@ pub enum StartupData<'a> { } type JSErrorCreateFn = dyn Fn(V8Exception) -> ErrBox; +type IsolateErrorHandleFn = dyn FnMut(ErrBox) -> Result<(), ErrBox>; /// A single execution context of JavaScript. Corresponds roughly to the "Web /// Worker" concept in the DOM. An Isolate is a Future that can be used with @@ -180,6 +182,7 @@ pub struct Isolate { startup_script: Option, pub op_registry: Arc, waker: AtomicWaker, + error_handler: Option>, } unsafe impl Send for Isolate {} @@ -246,9 +249,14 @@ impl Isolate { startup_script, op_registry: Arc::new(OpRegistry::new()), waker: AtomicWaker::new(), + error_handler: None, } } + pub fn set_error_handler(&mut self, handler: Box) { + self.error_handler = Some(handler); + } + /// Defines the how Deno.core.dispatch() acts. /// Called whenever Deno.core.dispatch() is called in JavaScript. zero_copy_buf /// corresponds to the second argument of Deno.core.dispatch(). @@ -402,17 +410,30 @@ impl Isolate { self.check_last_exception() } - fn check_last_exception(&self) -> Result<(), ErrBox> { + fn check_last_exception(&mut self) -> Result<(), ErrBox> { let ptr = unsafe { libdeno::deno_last_exception(self.libdeno_isolate) }; if ptr.is_null() { Ok(()) } else { let js_error_create = &*self.js_error_create; let cstr = unsafe { CStr::from_ptr(ptr) }; - let json_str = cstr.to_str().unwrap(); - let v8_exception = V8Exception::from_json(json_str).unwrap(); - let js_error = js_error_create(v8_exception); - Err(js_error) + if self.error_handler.is_some() { + // We duplicate the string and assert ownership. + // This is due to we want the user to safely clone the error. + let cstring = unsafe { CString::from_raw(strdup(ptr)) }; + // We need to clear last exception to avoid double handling. + unsafe { libdeno::deno_clear_last_exception(self.libdeno_isolate) }; + let json_string = cstring.into_string().unwrap(); + let v8_exception = V8Exception::from_json(&json_string).unwrap(); + let js_error = js_error_create(v8_exception); + let handler = self.error_handler.as_mut().unwrap(); + handler(js_error) + } else { + let json_str = cstr.to_str().unwrap(); + let v8_exception = V8Exception::from_json(json_str).unwrap(); + let js_error = js_error_create(v8_exception); + Err(js_error) + } } } @@ -445,7 +466,7 @@ impl Isolate { /// Low-level module creation. pub fn mod_new( - &self, + &mut self, main: bool, name: &str, source: &str, @@ -484,7 +505,7 @@ impl Isolate { /// ErrBox can be downcast to a type that exposes additional information about /// the V8 exception. By default this type is CoreJSError, however it may be a /// different type if Isolate::set_js_error_create() has been used. - pub fn snapshot(&self) -> Result, ErrBox> { + pub fn snapshot(&mut self) -> Result, ErrBox> { let snapshot = unsafe { libdeno::deno_snapshot_new(self.libdeno_isolate) }; match self.check_last_exception() { Ok(..) => Ok(snapshot), @@ -497,7 +518,7 @@ impl Isolate { } fn dyn_import_done( - &self, + &mut self, id: libdeno::deno_dyn_import_id, result: Result>, ) -> Result<(), ErrBox> { diff --git a/core/libdeno.rs b/core/libdeno.rs index a3bf0d66b1..d83c11196c 100644 --- a/core/libdeno.rs +++ b/core/libdeno.rs @@ -263,6 +263,7 @@ extern "C" { pub fn deno_new(config: deno_config) -> *const isolate; pub fn deno_delete(i: *const isolate); pub fn deno_last_exception(i: *const isolate) -> *const c_char; + pub fn deno_clear_last_exception(i: *const isolate); pub fn deno_check_promise_errors(i: *const isolate); pub fn deno_lock(i: *const isolate); pub fn deno_unlock(i: *const isolate); diff --git a/core/libdeno/api.cc b/core/libdeno/api.cc index 1ded39e1b3..08e8a6de98 100644 --- a/core/libdeno/api.cc +++ b/core/libdeno/api.cc @@ -138,6 +138,12 @@ const char* deno_last_exception(Deno* d_) { } } +void deno_clear_last_exception(Deno* d_) { + auto* d = deno::unwrap(d_); + d->last_exception_.clear(); + d->last_exception_handle_.Reset(); +} + void deno_execute(Deno* d_, void* user_data, const char* js_filename, const char* js_source) { auto* d = deno::unwrap(d_); diff --git a/core/libdeno/deno.h b/core/libdeno/deno.h index 946978e3b2..fd48abc5a5 100644 --- a/core/libdeno/deno.h +++ b/core/libdeno/deno.h @@ -117,8 +117,14 @@ void deno_pinned_buf_delete(deno_pinned_buf* buf); void deno_check_promise_errors(Deno* d); +// Returns a cstring pointer to the exception. +// Rust side must NOT assert ownership. const char* deno_last_exception(Deno* d); +// Clears last exception. +// Rust side must NOT hold pointer to exception string when called. +void deno_clear_last_exception(Deno* d_); + void deno_terminate_execution(Deno* d); void deno_run_microtasks(Deno* d, void* user_data); diff --git a/deno_typescript/lib.rs b/deno_typescript/lib.rs index e53a4243d8..c410c870fb 100644 --- a/deno_typescript/lib.rs +++ b/deno_typescript/lib.rs @@ -220,7 +220,7 @@ pub fn mksnapshot_bundle_ts( } fn write_snapshot( - runtime_isolate: Isolate, + mut runtime_isolate: Isolate, bundle: &Path, ) -> Result<(), ErrBox> { println!("creating snapshot...");