diff options
author | Kamil Ogórek <kamil.ogorek@gmail.com> | 2022-12-21 01:18:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-21 01:18:49 +0100 |
commit | 6a47ffa4d3f180c4807ca17594939b765e15751f (patch) | |
tree | 0822d7be771008b92991bb526beff89ad4f1dd62 | |
parent | 423474caa88f658477ed9f2df1099b7037166c3a (diff) |
fix(core): Do not print errors prop for non-AggregateError errors (#17123)
This commit fixes formatting of JSError with "errors" property. Before this
commit all instances of "Error" were treated as if they were "AggregateError"
if they had "errors" property. After this commit only actual instances of
"AggregateError" are formatted in such a way, while instances of "Error"
that have "errors" property are formatted without showing details of "errors".
-rw-r--r-- | cli/tests/run_tests.rs | 7 | ||||
-rw-r--r-- | cli/tests/testdata/run/error_with_errors_prop.js | 10 | ||||
-rw-r--r-- | cli/tests/testdata/run/error_with_errors_prop.js.out | 10 | ||||
-rw-r--r-- | core/error.rs | 67 |
4 files changed, 79 insertions, 15 deletions
diff --git a/cli/tests/run_tests.rs b/cli/tests/run_tests.rs index 6196f641d..092546aaf 100644 --- a/cli/tests/run_tests.rs +++ b/cli/tests/run_tests.rs @@ -2780,6 +2780,13 @@ mod run { exit_code: 1, }); + // Regression test for https://github.com/denoland/deno/issues/16340. + itest!(error_with_errors_prop { + args: "run --quiet run/error_with_errors_prop.js", + output: "run/error_with_errors_prop.js.out", + exit_code: 1, + }); + // Regression test for https://github.com/denoland/deno/issues/12143. itest!(js_root_with_ts_check { args: "run --quiet --check run/js_root_with_ts_check.js", diff --git a/cli/tests/testdata/run/error_with_errors_prop.js b/cli/tests/testdata/run/error_with_errors_prop.js new file mode 100644 index 000000000..d1c6bbfaa --- /dev/null +++ b/cli/tests/testdata/run/error_with_errors_prop.js @@ -0,0 +1,10 @@ +const error = new Error("Error with errors prop."); +error.errors = [ + new Error("Error message 1."), + new Error("Error message 2."), +]; +console.log(error.stack); +console.log(); +console.log(error); +console.log(); +throw error; diff --git a/cli/tests/testdata/run/error_with_errors_prop.js.out b/cli/tests/testdata/run/error_with_errors_prop.js.out new file mode 100644 index 000000000..3154e86e6 --- /dev/null +++ b/cli/tests/testdata/run/error_with_errors_prop.js.out @@ -0,0 +1,10 @@ +Error: Error with errors prop. + at [WILDCARD]/error_with_errors_prop.js:1:15 + +Error: Error with errors prop. + at [WILDCARD]/error_with_errors_prop.js:1:15 + +error: Uncaught Error: Error with errors prop. +const error = new Error("Error with errors prop."); + ^ + at [WILDCARD]/error_with_errors_prop.js:1:15 diff --git a/core/error.rs b/core/error.rs index dc50b4d73..4ee3b9315 100644 --- a/core/error.rs +++ b/core/error.rs @@ -326,6 +326,7 @@ impl JsError { } if is_instance_of_error(scope, exception) { + let v8_exception = exception; // The exception is a JS Error object. let exception: v8::Local<v8::Object> = exception.try_into().unwrap(); let cause = get_property(scope, exception, "cause"); @@ -444,24 +445,25 @@ impl JsError { } } - // Read an array of stored errors, this is only defined for `AggregateError` - let aggregated_errors = get_property(scope, exception, "errors"); - let aggregated_errors: Option<v8::Local<v8::Array>> = - aggregated_errors.and_then(|a| a.try_into().ok()); - let mut aggregated: Option<Vec<JsError>> = None; - - if let Some(errors) = aggregated_errors { - if errors.length() > 0 { - let mut agg = vec![]; - for i in 0..errors.length() { - let error = errors.get_index(scope, i).unwrap(); - let js_error = Self::from_v8_exception(scope, error); - agg.push(js_error); + if is_aggregate_error(scope, v8_exception) { + // Read an array of stored errors, this is only defined for `AggregateError` + let aggregated_errors = get_property(scope, exception, "errors"); + let aggregated_errors: Option<v8::Local<v8::Array>> = + aggregated_errors.and_then(|a| a.try_into().ok()); + + if let Some(errors) = aggregated_errors { + if errors.length() > 0 { + let mut agg = vec![]; + for i in 0..errors.length() { + let error = errors.get_index(scope, i).unwrap(); + let js_error = Self::from_v8_exception(scope, error); + agg.push(js_error); + } + aggregated = Some(agg); } - aggregated = Some(agg); } - } + }; Self { name: e.name, @@ -575,6 +577,41 @@ pub(crate) fn is_instance_of_error<'s>( false } +/// Implements `value instanceof primordials.AggregateError` in JS, +/// by walking the prototype chain, and comparing each links constructor `name` property. +/// +/// NOTE: There is currently no way to detect `AggregateError` via `rusty_v8`, +/// as v8 itself doesn't expose `v8__Exception__AggregateError`, +/// and we cannot create bindings for it. This forces us to rely on `name` inference. +pub(crate) fn is_aggregate_error<'s>( + scope: &mut v8::HandleScope<'s>, + value: v8::Local<v8::Value>, +) -> bool { + let mut maybe_prototype = Some(value); + while let Some(prototype) = maybe_prototype { + if !prototype.is_object() { + return false; + } + + let prototype = prototype.to_object(scope).unwrap(); + let prototype_name = match get_property(scope, prototype, "constructor") { + Some(constructor) => { + let ctor = constructor.to_object(scope).unwrap(); + get_property(scope, ctor, "name").map(|v| v.to_rust_string_lossy(scope)) + } + None => return false, + }; + + if prototype_name == Some(String::from("AggregateError")) { + return true; + } + + maybe_prototype = prototype.get_prototype(scope); + } + + false +} + const DATA_URL_ABBREV_THRESHOLD: usize = 150; pub fn format_file_name(file_name: &str) -> String { |