diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 0e1a39deb5..223c02e244 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -425,6 +425,12 @@ itest!(uncaught_errors { exit_code: 1, }); +itest!(report_error { + args: "test --quiet test/report_error.ts", + output: "test/report_error.out", + exit_code: 1, +}); + itest!(check_local_by_default { args: "test --quiet test/check_local_by_default.ts", output: "test/check_local_by_default.out", diff --git a/cli/tests/testdata/test/report_error.out b/cli/tests/testdata/test/report_error.out new file mode 100644 index 0000000000..698550f97d --- /dev/null +++ b/cli/tests/testdata/test/report_error.out @@ -0,0 +1,23 @@ +running 2 tests from [WILDCARD]/report_error.ts +foo ... +Uncaught error from [WILDCARD]/report_error.ts FAILED +foo ... cancelled (0ms) +bar ... cancelled (0ms) + + ERRORS + +[WILDCARD]/report_error.ts (uncaught error) +error: Error: foo + reportError(new Error("foo")); + ^ + at [WILDCARD]/report_error.ts:2:15 +This error was not caught from a test and caused the test runner to fail on the referenced module. +It most likely originated from a dangling promise, event/timeout handler or top-level code. + + FAILURES + +[WILDCARD]/report_error.ts (uncaught error) + +FAILED | 0 passed | 3 failed ([WILDCARD]) + +error: Test failed diff --git a/cli/tests/testdata/test/report_error.ts b/cli/tests/testdata/test/report_error.ts new file mode 100644 index 0000000000..56b6db26c8 --- /dev/null +++ b/cli/tests/testdata/test/report_error.ts @@ -0,0 +1,6 @@ +Deno.test("foo", () => { + reportError(new Error("foo")); + console.log(1); +}); + +Deno.test("bar", () => {}); diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs index 962b1ac174..5f467bc6e2 100644 --- a/cli/tools/bench.rs +++ b/cli/tools/bench.rs @@ -489,14 +489,7 @@ async fn bench_specifier( }))?; for (desc, function) in benchmarks { sender.send(BenchEvent::Wait(desc.id))?; - let promise = { - let scope = &mut worker.js_runtime.handle_scope(); - let cb = function.open(scope); - let this = v8::undefined(scope).into(); - let promise = cb.call(scope, this, &[]).unwrap(); - v8::Global::new(scope, promise) - }; - let result = worker.js_runtime.resolve_value(promise).await?; + let result = worker.js_runtime.call_and_await(&function).await?; let scope = &mut worker.js_runtime.handle_scope(); let result = v8::Local::new(scope, result); let result = serde_v8::from_v8::(scope, result)?; diff --git a/cli/tools/test.rs b/cli/tools/test.rs index 268f3b4b9e..62a104733d 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -997,14 +997,7 @@ pub async fn test_specifier( } sender.send(TestEvent::Wait(desc.id))?; let earlier = SystemTime::now(); - let promise = { - let scope = &mut worker.js_runtime.handle_scope(); - let cb = function.open(scope); - let this = v8::undefined(scope).into(); - let promise = cb.call(scope, this, &[]).unwrap(); - v8::Global::new(scope, promise) - }; - let result = match worker.js_runtime.resolve_value(promise).await { + let result = match worker.js_runtime.call_and_await(&function).await { Ok(r) => r, Err(error) => { if error.is::() { diff --git a/core/inspector.rs b/core/inspector.rs index c83784fe38..b0a55cf12b 100644 --- a/core/inspector.rs +++ b/core/inspector.rs @@ -11,7 +11,6 @@ use crate::futures::channel::mpsc::UnboundedSender; use crate::futures::channel::oneshot; use crate::futures::future::select; use crate::futures::future::Either; -use crate::futures::future::Future; use crate::futures::prelude::*; use crate::futures::stream::SelectAll; use crate::futures::stream::StreamExt; @@ -82,6 +81,7 @@ pub struct JsRuntimeInspector { flags: RefCell, waker: Arc, deregister_tx: Option>, + is_dispatching_message: RefCell, } impl Drop for JsRuntimeInspector { @@ -141,18 +141,6 @@ impl v8::inspector::V8InspectorClientImpl for JsRuntimeInspector { } } -/// Polling `JsRuntimeInspector` allows inspector to accept new incoming -/// connections and "pump" messages in different sessions. -/// -/// It should be polled on tick of event loop, ie. in `JsRuntime::poll_event_loop` -/// function. -impl Future for JsRuntimeInspector { - type Output = (); - fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<()> { - self.poll_sessions(Some(cx)).unwrap() - } -} - impl JsRuntimeInspector { /// Currently Deno supports only a single context in `JsRuntime` /// and thus it's id is provided as an associated contant. @@ -182,6 +170,7 @@ impl JsRuntimeInspector { flags: Default::default(), waker, deregister_tx: None, + is_dispatching_message: Default::default(), })); let mut self_ = self__.borrow_mut(); self_.v8_inspector = Rc::new(RefCell::new( @@ -224,6 +213,10 @@ impl JsRuntimeInspector { self__ } + pub fn is_dispatching_message(&self) -> bool { + *self.is_dispatching_message.borrow() + } + pub fn context_destroyed( &mut self, scope: &mut HandleScope, @@ -246,7 +239,7 @@ impl JsRuntimeInspector { self.sessions.borrow().has_blocking_sessions() } - fn poll_sessions( + pub fn poll_sessions( &self, mut invoker_cx: Option<&mut Context>, ) -> Result, BorrowMutError> { @@ -304,7 +297,9 @@ impl JsRuntimeInspector { match sessions.established.poll_next_unpin(cx) { Poll::Ready(Some(session_stream_item)) => { let (v8_session_ptr, msg) = session_stream_item; + *self.is_dispatching_message.borrow_mut() = true; InspectorSession::dispatch_message(v8_session_ptr, msg); + *self.is_dispatching_message.borrow_mut() = false; continue; } Poll::Ready(None) => break, diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index 6e8b2efda0..f4133f3b8e 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -713,22 +713,17 @@ fn op_dispatch_exception( ) { let state_rc = JsRuntime::state(scope); let mut state = state_rc.borrow_mut(); - state - .dispatched_exceptions - .push_front(v8::Global::new(scope, exception.v8_value)); - // Only terminate execution if there are no inspector sessions. - if state.inspector.is_none() { - scope.terminate_execution(); - return; - } + if let Some(inspector) = &state.inspector { + let inspector = inspector.borrow(); + // TODO(nayeemrmn): Send exception message to inspector sessions here. - // FIXME(bartlomieju): I'm not sure if this assumption is valid... Maybe when - // inspector is polling on pause? - if state.inspector().try_borrow().is_ok() { - scope.terminate_execution(); - } else { - // If the inspector is borrowed at this time, assume an inspector is active. + // This indicates that the op is being called from a REPL. Skip termination. + if inspector.is_dispatching_message() { + return; + } } + state.dispatched_exception = Some(v8::Global::new(scope, exception.v8_value)); + scope.terminate_execution(); } #[op(v8)] diff --git a/core/runtime.rs b/core/runtime.rs index 9ead489af7..d8355ae6d6 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -44,7 +44,6 @@ use smallvec::SmallVec; use std::any::Any; use std::cell::RefCell; use std::collections::HashMap; -use std::collections::VecDeque; use std::ffi::c_void; use std::option::Option; use std::pin::Pin; @@ -176,7 +175,7 @@ pub struct JsRuntimeState { /// instead of any other exceptions. // TODO(nayeemrmn): This is polled in `exception_to_err_result()` which is // flimsy. Try to poll it similarly to `pending_promise_rejections`. - pub(crate) dispatched_exceptions: VecDeque>, + pub(crate) dispatched_exception: Option>, pub(crate) inspector: Option>>, waker: AtomicWaker, } @@ -349,7 +348,7 @@ impl JsRuntime { op_state: op_state.clone(), waker: AtomicWaker::new(), have_unpolled_ops: false, - dispatched_exceptions: Default::default(), + dispatched_exception: None, // Some fields are initialized later after isolate is created inspector: None, global_realm: None, @@ -946,6 +945,27 @@ impl JsRuntime { ) } + /// Call a function. If it returns a promise, run the event loop until that + /// promise is settled. If the promise rejects or there is an uncaught error + /// in the event loop, return `Err(error)`. Or return `Ok()`. + pub async fn call_and_await( + &mut self, + function: &v8::Global, + ) -> Result, Error> { + let promise = { + let scope = &mut self.handle_scope(); + let cb = function.open(scope); + let this = v8::undefined(scope).into(); + let promise = cb.call(scope, this, &[]); + if promise.is_none() || scope.is_execution_terminating() { + let undefined = v8::undefined(scope).into(); + return exception_to_err_result(scope, undefined, false); + } + v8::Global::new(scope, promise.unwrap()) + }; + self.resolve_value(promise).await + } + /// Takes a snapshot. The isolate should have been created with will_snapshot /// set to true. /// @@ -1195,7 +1215,7 @@ impl JsRuntime { if has_inspector { // We poll the inspector first. - let _ = self.inspector().borrow_mut().poll_unpin(cx); + let _ = self.inspector().borrow().poll_sessions(Some(cx)).unwrap(); } self.pump_v8_message_loop()?; @@ -1518,19 +1538,14 @@ pub(crate) fn exception_to_err_result( // to use the exception that was passed to it rather than the exception that // was passed to this function. let state = state_rc.borrow(); - exception = state - .dispatched_exceptions - .back() - .map(|exception| v8::Local::new(scope, exception.clone())) - .unwrap_or_else(|| { - // Maybe make a new exception object. - if was_terminating_execution && exception.is_null_or_undefined() { - let message = v8::String::new(scope, "execution terminated").unwrap(); - v8::Exception::error(scope, message) - } else { - exception - } - }); + exception = if let Some(exception) = &state.dispatched_exception { + v8::Local::new(scope, exception.clone()) + } else if was_terminating_execution && exception.is_null_or_undefined() { + let message = v8::String::new(scope, "execution terminated").unwrap(); + v8::Exception::error(scope, message) + } else { + exception + }; } let mut js_error = JsError::from_v8_exception(scope, exception); @@ -1738,7 +1753,7 @@ impl JsRuntime { status = module.get_status(); let has_dispatched_exception = - !state_rc.borrow_mut().dispatched_exceptions.is_empty(); + state_rc.borrow_mut().dispatched_exception.is_some(); if has_dispatched_exception { // This will be overrided in `exception_to_err_result()`. let exception = v8::undefined(tc_scope).into(); @@ -2659,7 +2674,7 @@ pub mod tests { .execute_script_static( "filename.js", r#" - + var promiseIdSymbol = Symbol.for("Deno.core.internalPromiseId"); var p1 = Deno.core.opAsync("op_test", 42); var p2 = Deno.core.opAsync("op_test", 42); @@ -2715,7 +2730,7 @@ pub mod tests { "filename.js", r#" let control = 42; - + Deno.core.opAsync("op_test", control); async function main() { Deno.core.opAsync("op_test", control); @@ -2734,7 +2749,7 @@ pub mod tests { .execute_script_static( "filename.js", r#" - + const p = Deno.core.opAsync("op_test", 42); if (p[Symbol.for("Deno.core.internalPromiseId")] == undefined) { throw new Error("missing id on returned promise"); @@ -2751,7 +2766,7 @@ pub mod tests { .execute_script_static( "filename.js", r#" - + Deno.core.opAsync("op_test"); "#, ) @@ -2766,7 +2781,7 @@ pub mod tests { .execute_script_static( "filename.js", r#" - + let zero_copy_a = new Uint8Array([0]); Deno.core.opAsync2("op_test", null, zero_copy_a); "#, @@ -3928,7 +3943,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { .execute_script_static( "macrotasks_and_nextticks.js", r#" - + (async function () { const results = []; Deno.core.setMacrotaskCallback(() => { @@ -4166,7 +4181,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { "", format!( r#" - + globalThis.rejectValue = undefined; Deno.core.setPromiseRejectCallback((_type, _promise, reason) => {{ globalThis.rejectValue = `{realm_name}/${{reason}}`; @@ -4604,7 +4619,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { runtime.v8_isolate(), "", r#" - + (async function () { const buf = await Deno.core.opAsync("op_test", false); let err; @@ -4657,7 +4672,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { runtime.v8_isolate(), "", r#" - + var promise = Deno.core.opAsync("op_pending"); "#, ) @@ -4667,7 +4682,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { runtime.v8_isolate(), "", r#" - + var promise = Deno.core.opAsync("op_pending"); "#, )