Skip to content

Commit 4386bf7

Browse files
divybotlittledivy
andauthored
fix(ext/web): friendlier URLPattern construction errors (#35102)
## Problem When `URLPattern` construction fails, the underlying [`urlpattern`](https://crates.io/crates/urlpattern) crate surfaces an opaque message with no indication of which input value was bad or how to fix it. The common trigger (reported in #25965) is a file-router route like Fresh's `[:slug].tsx`, which expands to a `::slug` pathname: ``` error: Uncaught (in promise) TypeError: tokenizer error: invalid name; must be at least length 1 (at char 1) path: new URLPattern({ pathname }), ``` You can't tell from that which component (`pathname`) was at fault, what its value was, or what to do about it. ## Change Enrich the error thrown from `op_urlpattern_parse` (`ext/web/urlpattern.rs`): - Echo the offending component value. - Point a caret at the failing character — only for init objects with a single component set, since the crate's reported position is relative to a single parsed component string, not the whole constructor string. - Add a hint for the most common mistake: a `:` that does not start a valid named group. For `new URLPattern({ pathname: "/::slug" })` the message is now: ``` Failed to parse pathname from "/::slug": tokenizer error: invalid name; must be at least length 1 (at char 1) /::slug ^ hint: ":" starts a named group and must be followed by a name (a letter or "_", then letters, digits or "_"). To match a literal ":", escape it as "\:". ``` Valid patterns are unaffected, and the existing crate message is preserved as the leading line so nothing that parsed it loses information. ## Tests Added unit tests in `tests/unit/urlpattern_test.ts` covering both the init-object (with caret) and constructor-string (no caret) paths. WPT `urlpattern` tests assert only on the thrown error *type*, not its message, so they are unaffected. Closes #25965 Closes denoland/divybot#542 Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 05a686c commit 4386bf7

2 files changed

Lines changed: 135 additions & 4 deletions

File tree

ext/web/urlpattern.rs

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,99 @@ use deno_core::op2;
44
use serde::Serialize;
55
use urlpattern::quirks;
66
use urlpattern::quirks::StringOrInit;
7+
use urlpattern::quirks::UrlPatternInit;
78

8-
deno_error::js_error_wrapper!(urlpattern::Error, UrlPatternError, "TypeError");
9+
#[derive(Debug, thiserror::Error, deno_error::JsError)]
10+
#[class(type)]
11+
#[error("{0}")]
12+
pub struct UrlPatternError(String);
13+
14+
impl From<urlpattern::Error> for UrlPatternError {
15+
fn from(err: urlpattern::Error) -> Self {
16+
UrlPatternError(err.to_string())
17+
}
18+
}
19+
20+
/// Turns an opaque `urlpattern` crate error into an actionable message by
21+
/// echoing the offending pattern, pointing a caret at the failing character,
22+
/// and adding a hint for the most common mistake (a `:` that does not start a
23+
/// valid named group, e.g. file-router syntax like `[:slug]`).
24+
fn enrich_url_pattern_error(
25+
err: urlpattern::Error,
26+
input: &StringOrInit,
27+
) -> UrlPatternError {
28+
let message = err.to_string();
29+
30+
// Tokenizer errors carry the (char) position of the offending token.
31+
let pos = match err {
32+
urlpattern::Error::Tokenizer(_, pos) => Some(pos),
33+
_ => None,
34+
};
35+
36+
// Determine which pattern string the error refers to, and whether the
37+
// reported position reliably indexes into it. The crate parses each URL
38+
// component separately, so `pos` is relative to a single component string.
39+
// For an init object with exactly one component set we can attribute the
40+
// error to it and render a caret. For a constructor string `pos` is relative
41+
// to whichever component the string expanded to, not the whole string, so we
42+
// echo the input but cannot place a caret.
43+
let (pattern, caret_pos) = match input {
44+
StringOrInit::String(s) => (Some(("URLPattern", s.as_str())), None),
45+
StringOrInit::Init(init) => (single_init_component(init), pos),
46+
};
47+
48+
let Some((name, pattern)) = pattern else {
49+
return UrlPatternError(message);
50+
};
51+
52+
let mut out = format!("Failed to parse {name} from \"{pattern}\": {message}");
53+
54+
if let Some(pos) = caret_pos {
55+
out.push_str("\n\n ");
56+
out.push_str(pattern);
57+
out.push_str("\n ");
58+
// `pos` is a count of chars; pattern inputs are practically ASCII, so a
59+
// space per preceding char aligns the caret under the offending one.
60+
for _ in 0..pos {
61+
out.push(' ');
62+
}
63+
out.push('^');
64+
}
65+
66+
if message.contains("invalid name") {
67+
out.push_str(
68+
"\n\n hint: \":\" starts a named group and must be followed by a name \
69+
(a letter or \"_\", then letters, digits or \"_\"). To match a literal \
70+
\":\", escape it as \"\\:\".",
71+
);
72+
}
73+
74+
UrlPatternError(out)
75+
}
76+
77+
/// Returns the single set component of a `UrlPatternInit` (with its field name),
78+
/// or `None` if zero or more than one component is set.
79+
fn single_init_component(
80+
init: &UrlPatternInit,
81+
) -> Option<(&'static str, &str)> {
82+
let components: [(&'static str, &Option<String>); 8] = [
83+
("protocol", &init.protocol),
84+
("username", &init.username),
85+
("password", &init.password),
86+
("hostname", &init.hostname),
87+
("port", &init.port),
88+
("pathname", &init.pathname),
89+
("search", &init.search),
90+
("hash", &init.hash),
91+
];
92+
let mut set = components
93+
.iter()
94+
.filter_map(|(name, value)| value.as_deref().map(|value| (*name, value)));
95+
match (set.next(), set.next()) {
96+
(Some(only), None) => Some(only),
97+
_ => None,
98+
}
99+
}
9100

10101
/// Lean version of UrlPatternComponent that excludes the unused `matcher` field.
11102
#[derive(Serialize)]
@@ -49,9 +140,11 @@ pub fn op_urlpattern_parse(
49140
#[serde] options: urlpattern::UrlPatternOptions,
50141
) -> Result<UrlPatternResult, UrlPatternError> {
51142
let init =
52-
quirks::process_construct_pattern_input(input, base_url.as_deref())?;
143+
quirks::process_construct_pattern_input(input.clone(), base_url.as_deref())
144+
.map_err(|e| enrich_url_pattern_error(e, &input))?;
53145

54-
let pattern = quirks::parse_pattern(init, options)?;
146+
let pattern = quirks::parse_pattern(init, options)
147+
.map_err(|e| enrich_url_pattern_error(e, &input))?;
55148

56149
Ok(UrlPatternResult {
57150
protocol: pattern.protocol.into(),

tests/unit/urlpattern_test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Copyright 2018-2026 the Deno authors. MIT license.
2-
import { assert, assertEquals } from "./test_util.ts";
2+
import { assert, assertEquals, assertThrows } from "./test_util.ts";
33
import { assertType, IsExact } from "@std/testing/types";
44

55
Deno.test(function urlPatternFromString() {
@@ -73,3 +73,41 @@ Deno.test(function urlPatternIgnoreCase() {
7373
assert(p.test("/test", "http://localhost"));
7474
assert(p.test("/TeSt", "http://localhost"));
7575
});
76+
77+
Deno.test(function urlPatternInvalidNameError() {
78+
// A ":" not followed by a valid group name (e.g. file-router syntax such as
79+
// `[:slug]` expanding to `::slug`) should produce an actionable error that
80+
// echoes the offending component, points a caret at it, and includes a hint.
81+
const err = assertThrows(
82+
() => new URLPattern({ pathname: "/::slug" }),
83+
TypeError,
84+
) as TypeError;
85+
assert(
86+
err.message.includes('Failed to parse pathname from "/::slug"'),
87+
`missing component context: ${err.message}`,
88+
);
89+
assert(err.message.includes("^"), `missing caret: ${err.message}`);
90+
assert(
91+
err.message.includes('hint: ":" starts a named group'),
92+
`missing hint: ${err.message}`,
93+
);
94+
});
95+
96+
Deno.test(function urlPatternInvalidNameErrorFromString() {
97+
// For a constructor string we cannot reliably map the position back to the
98+
// full input, but the input and hint should still be surfaced.
99+
const err = assertThrows(
100+
() => new URLPattern("https://example.com/::slug"),
101+
TypeError,
102+
) as TypeError;
103+
assert(
104+
err.message.includes(
105+
'Failed to parse URLPattern from "https://example.com/::slug"',
106+
),
107+
`missing input context: ${err.message}`,
108+
);
109+
assert(
110+
err.message.includes('hint: ":" starts a named group'),
111+
`missing hint: ${err.message}`,
112+
);
113+
});

0 commit comments

Comments
 (0)