Skip to content

Commit

Permalink
repl: do not crash on async op reject (denoland#3527)
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinkassimo authored and ry committed Dec 20, 2019
1 parent fcae4a7 commit 9ef0b18
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 9 deletions.
5 changes: 5 additions & 0 deletions cli/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ impl Worker {
}
}

pub fn set_error_handler(
&mut self,
handler: Box<dyn FnMut(ErrBox) -> 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__");
Expand Down
37 changes: 29 additions & 8 deletions core/isolate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -180,6 +182,7 @@ pub struct Isolate {
startup_script: Option<OwnedScript>,
pub op_registry: Arc<OpRegistry>,
waker: AtomicWaker,
error_handler: Option<Box<IsolateErrorHandleFn>>,
}

unsafe impl Send for Isolate {}
Expand Down Expand Up @@ -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<IsolateErrorHandleFn>) {
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().
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -445,7 +466,7 @@ impl Isolate {

/// Low-level module creation.
pub fn mod_new(
&self,
&mut self,
main: bool,
name: &str,
source: &str,
Expand Down Expand Up @@ -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<Snapshot1<'static>, ErrBox> {
pub fn snapshot(&mut self) -> Result<Snapshot1<'static>, ErrBox> {
let snapshot = unsafe { libdeno::deno_snapshot_new(self.libdeno_isolate) };
match self.check_last_exception() {
Ok(..) => Ok(snapshot),
Expand All @@ -497,7 +518,7 @@ impl Isolate {
}

fn dyn_import_done(
&self,
&mut self,
id: libdeno::deno_dyn_import_id,
result: Result<deno_mod, Option<String>>,
) -> Result<(), ErrBox> {
Expand Down
1 change: 1 addition & 0 deletions core/libdeno.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions core/libdeno/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down
6 changes: 6 additions & 0 deletions core/libdeno/deno.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion deno_typescript/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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...");
Expand Down

0 comments on commit 9ef0b18

Please sign in to comment.