Report all hostname state failures for URLPattern

[annevk]

URLPattern's canonicalize a hostname is the only invocation of the basic URL parser with a URL and state override set to hostname state that looks at the return failure.

And since the URL that URLPattern uses is a dummy URL that is ideally not exposed, knowing about all the failure conditions is kind of important.

(Now technically the second return failure added here cannot be observed as the dummy URL won't have credentials or a non-null port, but it seemed good to change that at the same time for consistency.)

This is covered by existing URLPattern web-platform-tests. Some more background can be found in whatwg/urlpattern#252.

• At least two implementers are interested (and none opposed):
  • Required by URLPattern.
• Tests are written and can be reviewed and commented upon at:
  • URLPattern tests.
• Implementation bugs are filed:
  • Chromium: N/A, to be filed as part of URLPattern changes.
  • Gecko: N/A, to be filed as part of URLPattern changes.
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=289401
  • Deno: N/A, to be filed as part of URLPattern changes.
  • Node.js: N/A, to be filed as part of URLPattern changes.
• MDN issue is filed: N/A
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[annevk]

@anonrig @sisidovski what do you think of this approach?

[anonrig]

@anonrig @sisidovski what do you think of this approach?

It is kind of vague to me what the returning failure is. Can we be more explicit and standardize the error?

[annevk]

I'm not sure what you mean.

[sisidovski]

I assume @anonrig is talking about "failure" itself. In the spec returning "failure" in many places and the caller handles failures appropriately e.g.

If parseResult is failure, then throw a TypeError.

As the basic url parser is an internally called algorithm, it's totally fine to just return "failure" in failure cases as far as I understand it.

[rmisev]

You did not specify validation errors when returning failure. I think it might be useful to specify them.

[annevk]

We don't currently specify validation errors for API calls.

[shannonbooth]

Since port was mentioned, Is it intended that basic URL parsing invalid80 with state override set to Port will fail? See: https://github.com/web-platform-tests/wpt/blob/58257b113ea697d981808fed11daeaea30ab6bd1/urlpattern/resources/urlpatterntestdata.json#L1234

I guess the pattern spec was written with the intent of failure being returned here, but without URL spec doing it?

Currently spec will return a null port, the WPT test above seems to expect no match to happen by error being thrown during canonicalization.

[anonrig]

I can't approve this (due to insufficient permissions), but after reading it once again, I'm OK with landing it.

[annevk]

Thanks everyone! I'm going to land this as this is just a bug fix for whatwg/urlpattern#252. I suggest that as resolving that issue the remaining implementer bugs get filed.