From 504482dadd4d8cd9e4105d56ed86802906767f39 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 27 Apr 2023 22:36:49 +0100 Subject: [PATCH] fix(repl): print unhandled rejections and event errors (#18878) Fixes #8858. Fixes #8869. ``` $ target/debug/deno Deno 1.32.5 exit using ctrl+d, ctrl+c, or close() REPL is running with all permissions allowed. To specify permissions, run `deno repl` with allow flags. > Promise.reject(new Error("bar")); Promise { Error: bar at :2:16 } Uncaught (in promise) Error: bar at :2:16 > reportError(new Error("baz")); undefined Uncaught Error: baz at :2:13 > --- cli/tests/integration/repl_tests.rs | 30 ++++++++++++++++++++++++++++- cli/tools/repl/mod.rs | 19 +++++++++++++++++- cli/tools/repl/session.rs | 9 ++++----- core/inspector.rs | 29 ++++++++++++++++++++++++++++ core/ops_builtin_v8.rs | 3 +-- core/realm.rs | 10 ++++++++++ 6 files changed, 91 insertions(+), 9 deletions(-) diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index a473dc2006..d9966fe8ff 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -783,14 +783,42 @@ fn pty_tab_handler() { }); } +#[test] +fn repl_error() { + util::with_pty(&["repl"], |mut console| { + console.write_line("console.log(1);"); + console.expect_all(&["1", "undefined"]); + console.write_line(r#"throw new Error("foo");"#); + console.expect("Uncaught Error: foo"); + console.expect(" at "); + console.write_line("console.log(2);"); + console.expect("2"); + }); +} + +#[test] +fn repl_reject() { + util::with_pty(&["repl"], |mut console| { + console.write_line("console.log(1);"); + console.expect_all(&["1", "undefined"]); + console.write_line(r#"Promise.reject(new Error("foo"));"#); + console.expect("Promise { Error: foo"); + console.expect("Uncaught (in promise) Error: foo"); + console.expect(" at "); + console.write_line("console.log(2);"); + console.expect("2"); + }); +} + #[test] fn repl_report_error() { util::with_pty(&["repl"], |mut console| { console.write_line("console.log(1);"); console.expect_all(&["1", "undefined"]); - // TODO(nayeemrmn): The REPL should report event errors and rejections. console.write_line(r#"reportError(new Error("foo"));"#); console.expect("undefined"); + console.expect("Uncaught Error: foo"); + console.expect(" at "); console.write_line("console.log(2);"); console.expect("2"); }); diff --git a/cli/tools/repl/mod.rs b/cli/tools/repl/mod.rs index bfba627525..0a6d9b9e9d 100644 --- a/cli/tools/repl/mod.rs +++ b/cli/tools/repl/mod.rs @@ -7,6 +7,7 @@ use crate::colors; use crate::file_fetcher::FileFetcher; use crate::proc_state::ProcState; use deno_core::error::AnyError; +use deno_core::futures::StreamExt; use deno_runtime::permissions::Permissions; use deno_runtime::permissions::PermissionsContainer; use rustyline::error::ReadlineError; @@ -30,8 +31,11 @@ async fn read_line_and_poll( message_handler: &mut RustylineSyncMessageHandler, editor: ReplEditor, ) -> Result { + #![allow(clippy::await_holding_refcell_ref)] let mut line_fut = tokio::task::spawn_blocking(move || editor.readline()); let mut poll_worker = true; + let notifications_rc = repl_session.notifications.clone(); + let mut notifications = notifications_rc.borrow_mut(); loop { tokio::select! { @@ -57,7 +61,20 @@ async fn read_line_and_poll( } poll_worker = true; - }, + } + message = notifications.next() => { + if let Some(message) = message { + let method = message.get("method").unwrap().as_str().unwrap(); + if method == "Runtime.exceptionThrown" { + let params = message.get("params").unwrap().as_object().unwrap(); + let exception_details = params.get("exceptionDetails").unwrap().as_object().unwrap(); + let text = exception_details.get("text").unwrap().as_str().unwrap(); + let exception = exception_details.get("exception").unwrap().as_object().unwrap(); + let description = exception.get("description").unwrap().as_str().unwrap(); + println!("{text} {description}"); + } + } + } _ = repl_session.run_event_loop(), if poll_worker => { poll_worker = false; } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index b2645097c4..6f8db6fcd8 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -1,5 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::cell::RefCell; +use std::rc::Rc; use std::sync::Arc; use crate::args::CliOptions; @@ -128,12 +130,9 @@ pub struct ReplSession { session: LocalInspectorSession, pub context_id: u64, pub language_server: ReplLanguageServer, + pub notifications: Rc>>, has_initialized_node_runtime: bool, referrer: ModuleSpecifier, - // FIXME(bartlomieju): this field should be used to listen - // for "exceptionThrown" notifications - #[allow(dead_code)] - notification_rx: UnboundedReceiver, } impl ReplSession { @@ -193,7 +192,7 @@ impl ReplSession { language_server, has_initialized_node_runtime: false, referrer, - notification_rx, + notifications: Rc::new(RefCell::new(notification_rx)), }; // inject prelude diff --git a/core/inspector.rs b/core/inspector.rs index b0a55cf12b..22d1501544 100644 --- a/core/inspector.rs +++ b/core/inspector.rs @@ -231,6 +231,35 @@ impl JsRuntimeInspector { .context_destroyed(context); } + pub fn exception_thrown( + &self, + scope: &mut HandleScope, + exception: v8::Local<'_, v8::Value>, + in_promise: bool, + ) { + let context = scope.get_current_context(); + let message = v8::Exception::create_message(scope, exception); + let stack_trace = message.get_stack_trace(scope).unwrap(); + let mut v8_inspector_ref = self.v8_inspector.borrow_mut(); + let v8_inspector = v8_inspector_ref.as_mut().unwrap(); + let stack_trace = v8_inspector.create_stack_trace(stack_trace); + v8_inspector.exception_thrown( + context, + if in_promise { + v8::inspector::StringView::from("Uncaught (in promise)".as_bytes()) + } else { + v8::inspector::StringView::from("Uncaught".as_bytes()) + }, + exception, + v8::inspector::StringView::from("".as_bytes()), + v8::inspector::StringView::from("".as_bytes()), + 0, + 0, + stack_trace, + 0, + ); + } + pub fn has_active_sessions(&self) -> bool { self.sessions.borrow().has_active_sessions() } diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index f4133f3b8e..67cf1222f1 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -715,8 +715,7 @@ fn op_dispatch_exception( let mut state = state_rc.borrow_mut(); if let Some(inspector) = &state.inspector { let inspector = inspector.borrow(); - // TODO(nayeemrmn): Send exception message to inspector sessions here. - + inspector.exception_thrown(scope, exception.v8_value, false); // This indicates that the op is being called from a REPL. Skip termination. if inspector.is_dispatching_message() { return; diff --git a/core/realm.rs b/core/realm.rs index 08a550294d..f907553f08 100644 --- a/core/realm.rs +++ b/core/realm.rs @@ -4,6 +4,7 @@ use crate::bindings; use crate::modules::ModuleCode; use crate::ops::OpCtx; use crate::runtime::exception_to_err_result; +use crate::JsRuntime; use anyhow::Error; use std::cell::RefCell; use std::collections::HashMap; @@ -288,6 +289,15 @@ impl<'s> JsRealmLocal<'s> { drop(context_state); let exception = v8::Local::new(scope, handle); + let state_rc = JsRuntime::state(scope); + let state = state_rc.borrow(); + if let Some(inspector) = &state.inspector { + let inspector = inspector.borrow(); + inspector.exception_thrown(scope, exception, true); + if inspector.has_blocking_sessions() { + return Ok(()); + } + } exception_to_err_result(scope, exception, true) } }