diff --git a/changelog.d/9228-assert-regexp-matcher-input.md b/changelog.d/9228-assert-regexp-matcher-input.md new file mode 100644 index 0000000000..849fb2b2c6 --- /dev/null +++ b/changelog.d/9228-assert-regexp-matcher-input.md @@ -0,0 +1,32 @@ +`assert.throws(fn, /re/)` and friends now test the RegExp matcher the way Node +does: against `String(thrown)` only, with a RegExp treated as a terminal +matcher category. + +`expected_error_matches` used to give a non-matching pattern a second chance +against `thrown.message`, then fall through to the `instanceof` / +constructor-name / validation-function checks when that also failed. Two bugs +came out of one block: + +- A pattern that matched the bare message but not the stringified error was + wrongly accepted. `assert.throws(() => { throw new Error("nope") }, /^nope/)` + passed, where Node reports an `AssertionError` because `String(err)` is + `"Error: nope"`. Same for a thrown non-error carrying a `message` property. +- A pattern that matched nothing reached `js_instanceof_dynamic(thrown, regexp)` + and surfaced as a `TypeError` instead of an `AssertionError`, so + `assert.throws`/`rejects` reported the wrong error class and + `doesNotThrow`/`doesNotReject` failed to rethrow the original error. + +A RegExp *value on a validator key* (`{ message: /bad/ }`) is a different +matcher and still tests against that property. + +The pre-existing `assert/errors/strict-throws-validation.ts` could not catch +either bug: its pattern matched both the message and the stringified error, and +it only printed `name`/`code`. `assert/errors/throws-regexp-matcher-input.ts` +covers the two inputs separately across `throws`, `doesNotThrow`, `rejects`, +and `doesNotReject`. + +Known remaining gap: the generated `AssertionError` message is Perry's generic +"The thrown error did not match the expected matcher" rather than Node's +"The input did not match the regular expression /x/. Input:\n\n'Error: nope'\n". +That generic fallback is shared by every matcher category, so it is left for a +separate change. diff --git a/crates/perry-runtime/src/object/assert.rs b/crates/perry-runtime/src/object/assert.rs index 43a1b7a3df..f7dc77a7bf 100644 --- a/crates/perry-runtime/src/object/assert.rs +++ b/crates/perry-runtime/src/object/assert.rs @@ -259,14 +259,18 @@ fn expected_error_matches(thrown: f64, expected: f64) -> bool { if is_null_or_undefined(expected) { return true; } + // A RegExp matcher is a complete, terminal matcher category: Node tests it + // against `String(thrown)` and nothing else, and a non-match is an + // AssertionError — never a fallthrough to the instanceof / + // constructor-name / validation-function checks below (`instanceof` + // against a RegExp throws a TypeError). Retrying the pattern against + // `thrown.message` was a second, non-Node chance that wrongly accepted + // anchored patterns: `/^nope/` matched `new Error("nope")` even though + // `String(err)` is `"Error: nope"`. A RegExp *value on a validator key* + // (`{ message: /bad/ }`) is a different thing and is still tested against + // that property in `object_matcher_matches`. if let Some(matches_thrown) = regex_test_value(expected, thrown) { - if matches_thrown { - return true; - } - let message = read_property(thrown, "message"); - if !is_null_or_undefined(message) && regex_test_value(expected, message).unwrap_or(false) { - return true; - } + return matches_thrown; } // A plain object validator (e.g. `{ code: "ERR_X" }`) is a property-bag // matcher, never a constructor — its own enumerable keys must each equal @@ -1304,3 +1308,61 @@ pub extern "C" fn js_assert_if_error(value: f64) -> f64 { } throw_assertion(if_error_message(value), value, null_f64(), "ifError", false) } + +#[cfg(all(test, feature = "regex-engine"))] +mod regexp_matcher_tests { + use super::*; + + fn jsstr(bytes: &[u8]) -> *mut crate::StringHeader { + crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + } + + fn error_value(message: &[u8]) -> f64 { + let err = crate::error::js_error_new_with_message(jsstr(message)); + crate::value::js_nanbox_pointer(err as i64) + } + + fn regexp_value(pattern: &[u8]) -> f64 { + let re = crate::regex::js_regexp_new(jsstr(pattern), jsstr(b"")); + crate::value::js_nanbox_pointer(re as i64) + } + + /// Node tests a RegExp matcher against `String(thrown)` only. An anchored + /// pattern that matches the bare message must NOT match, because + /// `String(new Error("nope"))` is `"Error: nope"`. + #[test] + fn regexp_matcher_tests_the_stringified_error_not_the_message() { + let thrown = error_value(b"nope"); + assert!(!expected_error_matches(thrown, regexp_value(b"^nope"))); + assert!(expected_error_matches( + thrown, + regexp_value(b"^Error: nope$") + )); + } + + /// A RegExp is a terminal matcher category: a non-match returns `false` so + /// the caller reports an `AssertionError`, instead of falling through to + /// `js_instanceof_dynamic(thrown, regexp)`, which throws a `TypeError`. + #[test] + fn non_matching_regexp_is_terminal() { + assert!(!expected_error_matches( + error_value(b"nope"), + regexp_value(b"will-not-match") + )); + } + + /// A RegExp *value on a validator key* is a different matcher and is still + /// tested against that property, not against the stringified error. + #[test] + fn validator_key_regexp_still_tests_the_property() { + let thrown = error_value(b"bad value"); + let validator = crate::object::js_object_alloc(0, 1); + crate::object::js_object_set_field_by_name( + validator, + jsstr(b"message"), + regexp_value(b"^bad"), + ); + let validator = crate::value::js_nanbox_pointer(validator as i64); + assert!(expected_error_matches(thrown, validator)); + } +} diff --git a/test-parity/node-suite/assert/errors/throws-regexp-matcher-input.ts b/test-parity/node-suite/assert/errors/throws-regexp-matcher-input.ts new file mode 100644 index 0000000000..2d6d0885cc --- /dev/null +++ b/test-parity/node-suite/assert/errors/throws-regexp-matcher-input.ts @@ -0,0 +1,57 @@ +// A RegExp matcher is tested against `String(thrown)` and nothing else, and it +// is a terminal matcher category: a non-match is an AssertionError, never a +// fallthrough to the instanceof / constructor / validation-function checks. +// The pre-existing `strict-throws-validation.ts` only exercised a pattern that +// matches the message *and* the stringified error, so it could not tell the +// two inputs apart. +import assert from "node:assert"; + +function show(label: string, fn: () => void): void { + try { + fn(); + console.log(label + ": pass"); + } catch (err: any) { + console.log(label + ":", err?.name, err?.code ?? err?.operator ?? "no-code"); + } +} + +// `String(new Error("nope"))` is "Error: nope", so an anchored pattern that +// only matches the bare message must NOT match. +show("anchored message-only pattern", () => + assert.throws(() => { throw new Error("nope"); }, /^nope/)); + +// The same pattern anchored against the stringified error does match. +show("anchored stringified pattern", () => + assert.throws(() => { throw new Error("nope"); }, /^Error: nope$/)); + +// A plain object is stringified to "[object Object]"; its `message` property +// is not a second chance for the pattern. +show("plain object with message prop", () => + assert.throws(() => { throw { message: "nope" }; }, /^nope/)); + +// A thrown primitive stringifies to itself. +show("thrown string", () => + assert.throws(() => { throw "nope"; }, /^nope$/)); + +// doesNotThrow with a non-matching RegExp rethrows the original error rather +// than reporting an AssertionError (and must not raise a TypeError from a +// fallthrough into `instanceof`). +show("doesNotThrow non-matching rethrows", () => + assert.doesNotThrow(() => { throw new Error("nope"); }, /will-not-match/)); + +show("doesNotThrow matching reports", () => + assert.doesNotThrow(() => { throw new Error("nope"); }, /nope/)); + +// A RegExp value on a validator *key* is a different matcher: it is tested +// against that property, so an anchored message pattern does match there. +show("validator key regexp", () => + assert.throws(() => { throw new TypeError("bad value"); }, { message: /^bad/ })); + +// Async paths route through the same matcher. +await assert.rejects(async () => { throw new Error("nope"); }, /^nope/).then( + () => console.log("rejects anchored: pass"), + (err: any) => console.log("rejects anchored:", err?.name, err?.code ?? err?.operator)); + +await assert.doesNotReject(async () => { throw new Error("nope"); }, /will-not-match/).then( + () => console.log("doesNotReject non-matching: pass"), + (err: any) => console.log("doesNotReject non-matching:", err?.name, err?.code ?? "no-code"));