429 "Too Many Requests" HTTP code doesn't implement standard seconds value case

[NicolasMassart]

Description of the issue

Previously reported in tcort/markdown-link-check#114 this issue is in fact a link-check issue.

This issue has multiple parts linked together that have to be fixed at once.

Retry-After header format

The retry on 429 "Too Many Requests" HTTP code implemented in link-check expects a non standard retry format for the Retry-After optional header.
It looks for a duration of the form "1m30s" (for 1 minute 30 secondes) despite the standard being clear that the value format is seconds (or an http date format but I never saw that, so we may be safe to only have seconds)
See https://tools.ietf.org/html/rfc7231#section-7.1.3

Conversion error

But this would not be an issue if there was not also a conversion error.

For instance "Retry-After: 60" meaning wait for 60s before retry.

Link-check uses the Node.js ms library to converts in milliseconds.
It splits the string in parts to have "1m30s" look like "1m 30s" which is suitable for ms.
But ms is not applying any change on the value when it's already an integer.
ms(60) for 60 seconds from the "Retry-After: 60" becomes 60 milliseconds instead of 60000 and of course fails as it's too soon for the retry.

Fixing

We have to fix:

• accept the standard seconds format.
• convert "Retry-After" integer value in milliseconds.

Compatibility

Github recently fixed their issue that was returning "XmXXs" format retry headers to conform to the standard format in seconds and @sethmlarson says in the similar urllib3 issue:

We're going to close this issue as "not a defect in urllib3". GitHub needs to fix their headers.

But I think we should keep compatibility with the already implemented non standard way as it may impact some users.

We need to support "XmXXs" format and add the correct standard seconds support.

Testing

429 HTTP code testing must be added to the library and ensure that the retry delay value is well interpreted.
Test have to check that returning a "Retry-After:60" makes the second attempt wait for at least 60 seconds.

[sethmlarson]

I admire trying to keep backwards compatibility of broken servers but know that one library supporting non-standard behavior is often used to argue that other libraries should support that same non-standard behavior.

[NicolasMassart]

Yes I know. This is not a definitive statement.
I'm also in favour of not spreading bad habits but also, as a user, not to see my things break suddenly.

The reason behind this is that end users, those who use links-check and probably markdown-link-checker are not the people who have the possibility to change the wrong server behaviour.

They would have only two options:

1. Invest energy to make websites owners fix their non standard systems
2. Switch to another link checking tool.

Guess what? If I were these users I would choose 2.

So what I propose:

• Make an announcement to deprecate the non standard behaviour in a few versions.
• Display a warning for each link that returns an invalid Retry-After header and provide a link to an explanation about fixing this.
• After the deprecation delay, remove the non standard behaviour and list it as a breaking change in the release notes.