diff --git a/apps/cli/src/main.rs b/apps/cli/src/main.rs index 569d04f5..62ca0bb5 100644 --- a/apps/cli/src/main.rs +++ b/apps/cli/src/main.rs @@ -3,12 +3,14 @@ mod profiling; use quickjs_oxide::QUICKJS_COMPAT_VERSION; use quickjs_oxide::engine::api::{ Context, DebugInfoMode, DescriptorField, JsString, ModuleImportAttributes, - ModuleImportMetaProperty, ModuleLoadResult, ModuleLoader, ModuleLoaderError, - OrdinaryPropertyDescriptor, PromiseState, Runtime, RuntimeError, Value, number_to_string, - quickjs_detect_module_bytes, + ModuleImportMetaProperty, ModuleLoadResult, ModuleLoader, ModuleLoaderError, ObjectRef, + OrdinaryPropertyDescriptor, PromiseRejectionEvent, PromiseState, Runtime, RuntimeError, Value, + number_to_string, quickjs_detect_module_bytes, }; +use std::cell::RefCell; use std::io::Write as _; use std::process::ExitCode; +use std::rc::Rc; #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] enum SourceGoal { @@ -52,15 +54,20 @@ impl ModuleLoader for FileModuleLoader { fn load( &self, - _context: &mut quickjs_oxide::engine::api::Context, + context: &mut quickjs_oxide::engine::api::Context, normalized_name: &JsString, attributes: &ModuleImportAttributes, ) -> Result { let units = normalized_name.utf16_units().collect::>(); let filename = String::from_utf16(&units) .map_err(|_| ModuleLoaderError::new("module filename is not valid Unicode"))?; - let source = std::fs::read(&filename) - .map_err(|_| ModuleLoaderError::new(format!("module filename '{filename}'")))?; + let source = std::fs::read(&filename).map_err(|_| { + module_load_failure(context, &filename).unwrap_or_else(|error| { + ModuleLoaderError::new(format!( + "module filename '{filename}': host error construction failed: {error}" + )) + }) + })?; if import_type_is(attributes, "json5") { return Ok(ModuleLoadResult::Json5Bytes(source)); } @@ -76,6 +83,23 @@ impl ModuleLoader for FileModuleLoader { } } +/// Build the exact JavaScript `ReferenceError` that upstream's +/// `js_module_loader` throws when `js_load_file` fails +/// (`quickjs-libc.c:699`: "could not load module filename '%s'"). The host +/// loader raises the final text itself, so the engine-side dynamic-import +/// wrapper is never reached and the message is not doubled. Returning it as +/// [`ModuleLoaderError::exception`] preserves object identity through rejection. +fn module_load_failure( + context: &mut quickjs_oxide::engine::api::Context, + filename: &str, +) -> Result { + let error = context.new_native_error( + quickjs_oxide::engine::api::error::NativeErrorKind::Reference, + &format!("could not load module filename '{filename}'"), + )?; + Ok(ModuleLoaderError::exception(error)) +} + fn import_type_is(attributes: &ModuleImportAttributes, expected: &str) -> bool { attributes.effective().is_some_and(|attributes| { attributes.iter().any(|attribute| { @@ -120,6 +144,7 @@ fn main() -> ExitCode { let args = std::env::args().skip(1).collect::>(); let mut profile = profiling::Options::default(); let mut debug_info = DebugInfoMode::Full; + let mut dump_unhandled_promise_rejection = true; let mut expression = None; let mut print_result = false; let mut quit = false; @@ -131,6 +156,7 @@ fn main() -> ExitCode { match option.as_str() { "--" => break, "--strip-source" => debug_info = DebugInfoMode::StripSource, + "--no-unhandled-rejection" => dump_unhandled_promise_rejection = false, "--print-result" => print_result = true, "--module" => source_goal = SourceGoal::Module, "--script" => source_goal = SourceGoal::Script, @@ -149,6 +175,7 @@ fn main() -> ExitCode { println!(" --script load as a script"); println!(" -s strip all debug information"); println!(" --strip-source strip only function source text"); + println!(" --no-unhandled-rejection ignore unhandled promise rejections"); println!(" --print-result print the script completion value"); println!(" -v, --version show version and compatibility target"); profiling::help(); @@ -216,6 +243,9 @@ fn main() -> ExitCode { println!(" --script load as a script"); println!(" -s strip all debug information"); println!(" --strip-source strip only function source text"); + println!( + " --no-unhandled-rejection ignore unhandled promise rejections" + ); println!(" -v, --version show version and compatibility target"); profiling::help(); return ExitCode::SUCCESS; @@ -289,8 +319,11 @@ fn main() -> ExitCode { source_goal, main_module_path, &args[index..], - debug_info, - print_result, + HostOptions { + debug_info, + print_result, + dump_unhandled_promise_rejection, + }, &profile, ); } @@ -314,8 +347,11 @@ fn main() -> ExitCode { source_goal, Some(file), &args[index..], - debug_info, - print_result, + HostOptions { + debug_info, + print_result, + dump_unhandled_promise_rejection, + }, &profile, ) } @@ -332,6 +368,15 @@ enum EvaluationSource<'a> { Bytes(&'a [u8]), } +/// Process-wide host policies `evaluate` installs on the runtime, mirroring the +/// qjs.c flags of the same names. +#[derive(Clone, Copy)] +struct HostOptions { + debug_info: DebugInfoMode, + print_result: bool, + dump_unhandled_promise_rejection: bool, +} + #[allow(clippy::too_many_arguments)] fn evaluate( source: EvaluationSource<'_>, @@ -339,8 +384,7 @@ fn evaluate( source_goal: SourceGoal, main_module_path: Option<&str>, script_args: &[String], - debug_info: DebugInfoMode, - print_result: bool, + options: HostOptions, profile: &profiling::Options, ) -> ExitCode { // Declared before runtime so trace serialization happens after teardown. @@ -352,10 +396,28 @@ fn evaluate( } }; let runtime = session.runtime(); - runtime.set_debug_info_mode(debug_info); + runtime.set_debug_info_mode(options.debug_info); // Upstream qjs installs its filesystem loader for every process, including // Script-goal `-e`, so dynamic import has the same host boundary everywhere. let _module_loader = runtime.set_module_loader(FileModuleLoader); + // Host promise rejection tracking, mirroring + // `js_std_promise_rejection_tracker` in quickjs-libc.c: a rejection is + // rooted in publication order and its entry is dropped as soon as the + // host observes a handler being attached. Entries that survive the job + // drain are reported after the queue settles. + let pending_rejections: Rc>> = Rc::default(); + if options.dump_unhandled_promise_rejection { + let pending_rejections = Rc::clone(&pending_rejections); + runtime.set_host_promise_rejection_tracker(move |event: PromiseRejectionEvent| { + let mut pending = pending_rejections.borrow_mut(); + let promise = event.promise(); + if event.is_handled() { + pending.retain(|(candidate, _)| candidate != promise); + } else if !pending.iter().any(|(candidate, _)| candidate == promise) { + pending.push((promise.clone(), event.reason().clone())); + } + }); + } let mut context = runtime.new_context(); let snapshot = session.snapshot_guard(&runtime); let script_args = match script_args @@ -386,25 +448,31 @@ fn evaluate( }; match evaluation { Ok(value) => { - loop { - match runtime - .execute_pending_job() - .map_err(|error| error.into_error()) - { - Ok(outcome) if outcome.executed() => {} - Ok(_) => break, - Err(RuntimeError::Exception) => { - report_exception(format_pending_exception(&runtime, &mut context)); - return ExitCode::from(1); - } - Err(error) => { - eprintln!("{error}"); - return ExitCode::from(1); + if !drain_pending_jobs(&runtime, &mut context) { + return ExitCode::from(1); + } + snapshot.phase("after-jobs-before-context-drop"); + // `js_std_promise_rejection_check`: report every rejection that + // remained unhandled once the job queue drained, then exit 1. + let pending = std::mem::take(&mut *pending_rejections.borrow_mut()); + if !pending.is_empty() { + let stderr = std::io::stderr(); + let mut stderr = stderr.lock(); + for (_, reason) in &pending { + let _ = stderr.write_all(b"Possibly unhandled promise rejection: "); + match runtime.qjs_print_value_bytes(reason) { + Ok(diagnostic) => { + let _ = stderr.write_all(&diagnostic); + } + Err(_) => { + let _ = stderr.write_all(b"[unknown]"); + } } + let _ = stderr.write_all(b"\n"); } + return ExitCode::from(1); } - snapshot.phase("after-jobs-before-context-drop"); - if print_result { + if options.print_result { println!("{}", completion_text(value)); } ExitCode::SUCCESS @@ -432,6 +500,31 @@ fn is_module_file(filename: &str, source: &[u8]) -> bool { filename.ends_with(".mjs") || quickjs_detect_module_bytes(source) } +/// Drain the runtime job queue the way `js_std_loop` does. A job that leaves a +/// pending exception is reported with the ordinary uncaught-exception dump and +/// the queue keeps draining: the rejection of a derived Promise is routed into +/// its own reject capability by the reaction job, so such an exception is not a +/// fatal process error. Returns `false` when an internal host error (not a +/// JavaScript exception) aborted draining. +fn drain_pending_jobs(runtime: &Runtime, context: &mut Context) -> bool { + loop { + match runtime + .execute_pending_job() + .map_err(|error| error.into_error()) + { + Ok(outcome) if outcome.executed() => {} + Ok(_) => return true, + Err(RuntimeError::Exception) => { + report_exception(format_pending_exception(runtime, context)); + } + Err(error) => { + eprintln!("{error}"); + return false; + } + } + } +} + fn evaluate_module( runtime: &Runtime, context: &mut Context, diff --git a/apps/cli/tests/cli.rs b/apps/cli/tests/cli.rs index 87f968bb..9bf036fb 100644 --- a/apps/cli/tests/cli.rs +++ b/apps/cli/tests/cli.rs @@ -84,3 +84,6 @@ mod modules; #[path = "cli/options.rs"] mod options; + +#[path = "cli/rejections.rs"] +mod rejections; diff --git a/apps/cli/tests/cli/rejections.rs b/apps/cli/tests/cli/rejections.rs new file mode 100644 index 00000000..e893fdb8 --- /dev/null +++ b/apps/cli/tests/cli/rejections.rs @@ -0,0 +1,339 @@ +use super::*; + +fn run_file(options: &[&str], path: &Path) -> Output { + qjs() + .args(options) + .arg(cli_path(path)) + .output() + .expect("run qjs file") +} + +/// Host promise-rejection tracking (quickjs-libc.c +/// `js_std_promise_rejection_tracker` + `js_std_promise_rejection_check`): +/// rejections still without a handler once the job queue drains are printed to +/// stderr with the `Possibly unhandled promise rejection: ` prefix and the +/// process exits 1; attaching a handler before the drain removes the entry. +struct RejectionCase { + description: &'static str, + options: &'static [&'static str], + source: &'static str, + expected_status: i32, + expected_stderr: &'static [u8], +} + +const REJECTION_CASES: &[RejectionCase] = &[ + RejectionCase { + description: "an Error rejection reports its attached stack frame", + options: &[], + source: "Promise.reject(new Error(\"reject-boom\"));", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: reject-boom\n at (:1:25)\n", + }, + RejectionCase { + description: "an async function rejection reports its awaited frames", + options: &[], + source: "async function af(){ throw new Error(\"async-boom\"); } af();", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: async-boom\n at af (:1:37)\n at (:1:57)\n", + }, + RejectionCase { + description: "a string reason is quoted like JS_PrintValue", + options: &[], + source: "Promise.reject(\"s\");", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: \"s\"\n", + }, + RejectionCase { + description: "an undefined reason is printed verbatim", + options: &[], + source: "Promise.reject(undefined);", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: undefined\n", + }, + RejectionCase { + description: "a number reason is printed verbatim", + options: &[], + source: "Promise.reject(42);", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: 42\n", + }, + RejectionCase { + description: "primitive and object reasons print in publication order", + options: &[], + source: "Promise.reject(Symbol(\"s\")); Promise.reject(null); Promise.reject({a:1}); Promise.reject(123n);", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Symbol(s)\n\ +Possibly unhandled promise rejection: null\n\ +Possibly unhandled promise rejection: { a: 1 }\n\ +Possibly unhandled promise rejection: 123n\n", + }, + RejectionCase { + description: "a rejection handled before the drain stays silent", + options: &[], + source: "var p = Promise.reject(new Error(\"handled\")); p.catch(function(){});", + expected_status: 0, + expected_stderr: b"", + }, + RejectionCase { + description: "a handler attached from a pending job clears the entry", + options: &[], + source: "var p = Promise.reject(new Error(\"late\")); \ +Promise.resolve().then(function(){ p.catch(function(){}); });", + expected_status: 0, + expected_stderr: b"", + }, + RejectionCase { + description: "then with only onFulfilled derives a reported rejection", + options: &[], + source: "Promise.reject(new Error(\"down\")).then(function(v){});", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: down\n at (:1:25)\n", + }, + RejectionCase { + description: "a reaction-job throw and a direct rejection keep FIFO order", + options: &[], + source: "Promise.resolve().then(function(){ throw new Error(\"job-boom\"); }); \ +Promise.reject(\"r\");", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: \"r\"\n\ +Possibly unhandled promise rejection: Error: job-boom\n at (:1:51)\n", + }, + RejectionCase { + description: "a rejection created inside a reaction job is reported", + options: &[], + source: "Promise.resolve().then(function(){ Promise.reject(new Error(\"nested\")); });", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: nested\n at (:1:60)\n", + }, + RejectionCase { + description: "a catch callback returning a rejected promise is reported", + options: &[], + source: "Promise.reject(1).catch(function(){ return Promise.reject(new Error(\"rethrown\")); });", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: rethrown\n at (:1:68)\n", + }, + RejectionCase { + description: "a throwing then getter omits the resolving-function frame like QuickJS", + options: &[], + source: "Promise.resolve({get then(){throw new Error(\"gt\")}});", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: Error: gt\n at get then (:1:44)\n at resolve (native)\n at (:1:16)\n", + }, + RejectionCase { + description: "a failed dynamic import reports the loader's reference error verbatim", + options: &[], + source: "import(\"fixture-missing-xyz\");", + expected_status: 1, + expected_stderr: b"Possibly unhandled promise rejection: ReferenceError: could not load module filename 'fixture-missing-xyz'\n\n", + }, +]; + +#[test] +fn unhandled_promise_rejections_match_pinned_golden() { + for case in REJECTION_CASES { + let output = run_cli( + env!("CARGO_BIN_EXE_qjs").as_ref(), + case.options, + case.source, + case.description, + ); + assert_eq!( + output.status.code(), + Some(case.expected_status), + "{}: {}", + case.description, + String::from_utf8_lossy(&output.stderr), + ); + assert!(output.stdout.is_empty(), "{}", case.description); + assert_eq!(output.stderr, case.expected_stderr, "{}", case.description); + } +} + +#[test] +fn unhandled_promise_rejections_match_quickjs_oracle() { + let Some(oracle) = std::env::var_os("QJS_ORACLE") else { + eprintln!("SKIP promise-rejection differential: set QJS_ORACLE to upstream qjs"); + return; + }; + for case in REJECTION_CASES { + let quickjs = run_cli(&oracle, case.options, case.source, case.description); + let oxide = run_cli( + env!("CARGO_BIN_EXE_qjs").as_ref(), + case.options, + case.source, + case.description, + ); + assert_eq!( + oxide.status.code(), + quickjs.status.code(), + "{}", + case.description + ); + assert_eq!(oxide.stdout, quickjs.stdout, "{}", case.description); + assert_eq!(oxide.stderr, quickjs.stderr, "{}", case.description); + } +} + +#[test] +fn no_unhandled_rejection_flag_silences_the_report_and_exit_failure() { + let source = "Promise.reject(new Error(\"ignored\"));"; + + let silenced = run_cli( + env!("CARGO_BIN_EXE_qjs").as_ref(), + &["--no-unhandled-rejection"], + source, + "--no-unhandled-rejection silences the report", + ); + assert_eq!(silenced.status.code(), Some(0)); + assert!(silenced.stdout.is_empty()); + assert!(silenced.stderr.is_empty()); + + let reported = run_cli( + env!("CARGO_BIN_EXE_qjs").as_ref(), + &[], + source, + "default tracking reports the rejection", + ); + assert_eq!(reported.status.code(), Some(1)); + assert_eq!( + reported.stderr, + b"Possibly unhandled promise rejection: Error: ignored\n at (:1:25)\n" + ); +} + +#[test] +fn no_unhandled_rejection_flag_matches_quickjs_oracle() { + let Some(oracle) = std::env::var_os("QJS_ORACLE") else { + eprintln!("SKIP --no-unhandled-rejection differential: set QJS_ORACLE to upstream qjs"); + return; + }; + let source = "Promise.reject(new Error(\"ignored\"));"; + let quickjs = run_cli(&oracle, &["--no-unhandled-rejection"], source, "flag"); + let oxide = run_cli( + env!("CARGO_BIN_EXE_qjs").as_ref(), + &["--no-unhandled-rejection"], + source, + "flag", + ); + assert_eq!(oxide.status.code(), quickjs.status.code()); + assert_eq!(oxide.stdout, quickjs.stdout); + assert_eq!(oxide.stderr, quickjs.stderr); +} + +#[test] +fn module_evaluation_rejection_is_reported_even_when_import_is_handled() { + let fixture = ModuleFixture::new(); + fixture.write("b.mjs", "print('b ok');\n"); + fixture.write("a.mjs", "import './b.mjs';\nthrow new Error('a-fails');\n"); + let entry = fixture.write( + "entry.mjs", + "await import('./a.mjs').then(function(){ print('resolved'); }, \ +function(e){ print('rejected', e.name, e.message); });\n\ +print('entry continues');\n", + ); + + let oxide = run_file(&[], &entry); + assert_eq!(oxide.status.code(), Some(1)); + assert_eq!( + oxide.stdout, + b"b ok\nrejected Error a-fails\nentry continues\n" + ); + let diagnostic = String::from_utf8(oxide.stderr.clone()).unwrap(); + assert!( + diagnostic.starts_with("Possibly unhandled promise rejection: Error: a-fails\n"), + "{diagnostic}" + ); + assert!(diagnostic.contains("a.mjs:2:16"), "{diagnostic}"); + + let Some(oracle) = std::env::var_os("QJS_ORACLE") else { + eprintln!("SKIP module-rejection differential: set QJS_ORACLE to upstream qjs"); + return; + }; + let quickjs = Command::new(oracle) + .arg(cli_path(&entry)) + .output() + .expect("run QuickJS module rejection case"); + assert_eq!(oxide.status.code(), quickjs.status.code()); + assert_eq!(oxide.stdout, quickjs.stdout); + assert_eq!(oxide.stderr, quickjs.stderr); +} + +/// A2-1 (cross-family review): a dynamic import of a missing file from a +/// module must surface the host loader's `ReferenceError` +/// ("could not load module filename ''") exactly once. Previously the +/// engine re-wrapped the host message, doubling it. Covers both the rejection +/// line on stderr and the caught channel (name/message), byte-exact vs qjs. +#[test] +fn failed_dynamic_import_message_is_not_doubled() { + let fixture = ModuleFixture::new(); + let entry = fixture.write("entry.mjs", "import('./missing-xyz.mjs');\n"); + + // Golden: the phrase "could not load module" appears exactly once (no + // double wrap), and the line keeps the normalized specifier. + let unhandled = run_file(&[], &entry); + assert_eq!(unhandled.status.code(), Some(1)); + assert!(unhandled.stdout.is_empty()); + let report = String::from_utf8(unhandled.stderr).unwrap(); + assert!( + report.starts_with("Possibly unhandled promise rejection: "), + "{report}" + ); + assert!( + report.contains("could not load module filename '"), + "{report}" + ); + assert_eq!( + report.matches("could not load module").count(), + 1, + "{report}" + ); + // The loader error carries an empty `stack` (it is raised from a host + // callback without a JS frame), exactly like pinned qjs, so the report + // ends with two newlines. + assert!(report.ends_with("missing-xyz.mjs'\n\n"), "{report}"); + + // Caught channel: error.name/message survive unchanged, exit 0. The + // module goal normalizes the relative specifier to an absolute filename. + let missing_path = fixture.root.join("missing-xyz.mjs"); + let caught_entry = fixture.write( + "caught.mjs", + "import('./missing-xyz.mjs').catch(function(error) {\n\ + print(error.name);\n\ + print(error.message);\n\ + });\n", + ); + let caught = run_file(&[], &caught_entry); + assert!( + caught.status.success(), + "{}", + String::from_utf8_lossy(&caught.stderr) + ); + assert_eq!( + String::from_utf8(caught.stdout).unwrap(), + format!( + "ReferenceError\ncould not load module filename '{}'\n", + cli_path(&missing_path) + ) + ); + assert!(caught.stderr.is_empty()); + + let Some(oracle) = std::env::var_os("QJS_ORACLE") else { + eprintln!("SKIP module-load-failure differential: set QJS_ORACLE to upstream qjs"); + return; + }; + for path in [&entry, &caught_entry] { + let quickjs = Command::new(&oracle) + .arg(cli_path(path)) + .output() + .expect("run QuickJS module load failure case"); + let oxide = run_file(&[], path); + assert_eq!( + oxide.status.code(), + quickjs.status.code(), + "{}", + path.display() + ); + assert_eq!(oxide.stdout, quickjs.stdout, "{}", path.display()); + assert_eq!(oxide.stderr, quickjs.stderr, "{}", path.display()); + } +} diff --git a/src/engine/api/context/mod.rs b/src/engine/api/context/mod.rs index 346c1386..e6c538c9 100644 --- a/src/engine/api/context/mod.rs +++ b/src/engine/api/context/mod.rs @@ -98,4 +98,18 @@ impl Context { pub fn take_exception(&mut self) -> Result, RuntimeError> { self.runtime.take_pending_exception() } + + /// Construct a realm-intrinsic native error value (the same + /// `JS_ThrowReferenceError` / `JS_ThrowTypeError` … factory used by + /// built-ins), capturing a backtrace exactly as a thrown native error + /// would. This reads the realm's intrinsic constructor rather than the + /// mutable global binding, so host callbacks that need to raise a + /// specification-defined error are immune to global object tampering. + pub fn new_native_error( + &self, + kind: NativeErrorKind, + message: &str, + ) -> Result { + self.runtime.new_native_error(self.realm, kind, message) + } } diff --git a/src/engine/vm/frames.rs b/src/engine/vm/frames.rs index faddd847..aab16755 100644 --- a/src/engine/vm/frames.rs +++ b/src/engine/vm/frames.rs @@ -272,7 +272,7 @@ impl<'a> NativePublicationWitness<'a> { function, self.realm, ActiveFrameFlags { - backtrace_hidden: self.iterator_next_raw, + backtrace_hidden: self.backtrace_hidden(), ..Default::default() }, ActiveFrameKind::Native { @@ -283,6 +283,17 @@ impl<'a> NativePublicationWitness<'a> { continuation, ) } + + /// QuickJS invokes `JS_CLASS_PROMISE_RESOLVE_FUNCTION` / + /// `JS_CLASS_PROMISE_REJECT_FUNCTION` through the class call table without + /// pushing a `JSStackFrame` (`JS_CallInternal` hands non-bytecode classes + /// straight to their call handler), so errors raised while resolving a + /// thenable must not name the resolving function in their backtrace. + /// `Iterator.prototype.next` raw fast-path frames are hidden for the same + /// reason (its `%IteratorHelperPrototype%` frames are not observable). + fn backtrace_hidden(&self) -> bool { + self.iterator_next_raw || matches!(self.target, NativeFunctionId::PromiseResolving(_)) + } } impl Runtime {