Named validation errors

[TRowbotham]

This addresses #406.

By my count, there are 54 different places where a validation error can occur with the majority of them being unique. This adds a table with descriptive error codes for each unique validation error, which also has an associated description of the error.

• Would there by value in adding a column indicating whether the validation error can cause the parser to fail?
• I am considering adding example input that would trigger each validation error. Is this something you think would have value?
• The unexpected-windows-drive-letter-host needs an error description still.
• I wasn't sure how best to link the validation errors in each spot, so I just ended up doing <a><unique error name></a> <a>validation error</a> everywhere. I had considered a prefix or suffix of validation-error to every code, but felt it was overly verbose, though I'm not opposed to it if someone feels its more appropriate.
• "Error Message" should probably be renamed to "Error Description".
• Some of the error descriptions could probably use a little more detail, but I wanted to get this initial draft posted.

---

Preview | Diff

[domenic]

This looks amazing! Giving my opinions on some of your questions, but you should probably wait for @annevk before acting on them:

Would there by value in adding a column indicating whether the validation error can cause the parser to fail?

IMO yes. I'm thinking a "Fatal" column which is either empty or one of the unicode checkmarks.

I am considering adding example input that would trigger each validation error. Is this something you think would have value?

IMO yes. Doing so in a way that makes things non-overwhelming might be a bit tricky. E.g. I think another column would probably not work well. Maybe a second row (making each error code rowspan="2") with class="example", similar to https://html.spec.whatwg.org/#usage-summary-2 ? Or maybe just putting in <p class="example"> wouldn't be so bad.

Relatedly, although I imagine you are following the pattern from HTML, maybe the descriptions could be tightened up by removing "This error occurs when".

I wasn't sure how best to link the validation errors in each spot,

I like what you did. It provides a convenient link to the concept of validation error, which is important as people can be confused by parse error vs. validation error.

[TRowbotham]

Thanks for the feedback. I'll work on adding a new "Fatal" column as domenic suggested and rewording the descriptions to try not to repeat the "This error occurs when...".

I agree that an additional column probably isn't the best approach for the example input. The HTML parse errors table appears to put notes and examples right under the error description text, so I'll experiment with doing that and hopefully things don't get too cluttered.

[domenic]

The HTML parse errors table appears to put notes and examples right under the error description text, so I'll experiment with doing that and hopefully things don't get too cluttered.

One thing that will declutter things a bit is adding <p> around each description, which will introduce some nicer margins.

[annevk]

@TRowbotham if this needs rebasing at some point due to other ongoing changes I'm happy to do that work for you.

[TRowbotham]

I think I broke this while rebasing :(

[TRowbotham]

@annevk, I think this is ready now. It just needs a rebase, which I'll leave up to you so I don't botch it again.

[annevk]

@TRowbotham how are the rows ordered?

[TRowbotham]

They are grouped by section in the spec. I started at section 4.4 URL Parsing and worked my way backwards through the sections.

[annevk]

For port-out-of-range a problem is that URL-port string should define the upper limit. If it defines the upper limit, do we still want a separate enum value? I guess we do, but the description would have to change as it would match invalid-port otherwise.

[karwa]

I think the missing-credentials error is incorrect. That error occurs if there is an @ but no host. https://@test.com should not cause the parser to fail, but https://username:password@ should (and does, in the Live URL viewer in Safari 13).

Perhaps it should be replaced by something like 'unexpected credentials without host' (similar to 'unexpected port without host')?

[annevk]

The IPv4 parser still has two unclassified validation errors.

Otherwise this has been rebased. I adjusted casing to my liking. I addressed @karwa's observation. I want to give it another pass and address those missing cases, but it's getting late.

[annevk]

I want #739 to land first and rebase this on top.

I also want to divide the table into 3 row groups. Tentatively titled "Hosts", "URLs", and "Opaque hosts and URLs". And then put the errors in order of appearance.

[annevk]

This is now ready for review. All validation errors should be covered. Please don't hold back on nits!

[TRowbotham]

Thank you for driving this to completion!

Overall, I think this looks pretty good. I think the IPv4 and IPv6 validation error names could use a normalization pass. Currently, there are a few with similar names, but slightly different word order such as:

• too-many-IPv4-parts
• IPv6-too-many-pieces
• IPv4-in-IPv6-too-many-parts

[TRowbotham]

I stood up a small demo implementation, similar to the jsdom version, you can use it to quickly check validation errors if that helps with the review.

The git branch of the url parser the demo is using isn't published at the moment, but I can publish the branch if you would find the source code useful. The error highlighting could probably use some refinement, but it should do the job for reporting the validation error.

Edit: fixed the link

[annevk]

I pushed a large number of changes after a fairly lengthy editing session, but it's still not done unfortunately.

[annevk]

@TRowbotham did you want to give this a final look before it's merged?

[TRowbotham]

LGTM. Thank you for pushing this over the finish line!