Support 429 properly with Retry-After

[dtheodor]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429

429 (and 503) may carry the Retry-After header, which determines the retry delay. I think handling this should be built in this library by default, i.e. retry on 429 together with the 5xx family of errors, and check for Retry-After and use its value to override the timeout setting

[mawrkus]

Good idea, would gladly accept a PR for this.

[iDVB]

Is there a way to use retryCondition to support 429 as a manual change for now?

[mawrkus]

@iDVB I think you found the answer in https://github.com/softonic/axios-retry/issues/105, right?

[parsley72]

I looked at #105 but it doesn't seem to use the actual value returned for Retry-After. I wrote a simple test case around https://httpstat.us/429 which does set Retry-After:

$ curl -I https://httpstat.us/429
HTTP/2 429 
date: Mon, 31 Aug 2020 02:40:03 GMT
content-type: text/plain; charset=utf-8
content-length: 21
set-cookie: __cfduid=db8204c36b4786ae24b6946961e6f52941598841603; expires=Wed, 30-Sep-20 02:40:03 GMT; path=/; domain=.httpstat.us; HttpOnly; SameSite=Lax
cache-control: private
retry-after: 5
x-aspnetmvc-version: 5.1
access-control-allow-origin: *
x-aspnet-version: 4.0.30319
request-context: appId=cid-v1:7585021b-2db7-4da6-abff-2cf23005f0a9
access-control-expose-headers: Request-Context
x-powered-by: ASP.NET
set-cookie: ARRAffinity=c529441eca9a1f7cff05208d4b61553ecda3678bc13876939acfd3b19b1183c2;Path=/;HttpOnly;Domain=httpstat.us
cf-cache-status: DYNAMIC
cf-request-id: 04e3fbdccf0000eea62406e200000001
expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
server: cloudflare
cf-ray: 5cb362747eb7eea6-AKL

But Axios doesn't seem to present retry-after in headers. Is it just not being parsed?

[parsley72]

This is a CORS bug in httpstatus: https://github.com/Readify/httpstatus/issues/83

[parsley72]

Now I've got something consistent to test against this works:

  axiosRetry(axios, {
    retryCondition: (e) => {
      return (
        axiosRetry.isNetworkOrIdempotentRequestError(e) ||
        e.response.status === 429
      );
    },
    retryDelay: (retryCount, error) => {
      if (error.response) {
        const retry_after = error.response.headers["retry-after"];
        if (retry_after) {
          return retry_after;
        }
      }
      return axiosRetry.exponentialDelay(retryCount, error);
    }
  });

  axios
    .get("https://httpstat.us/200/cors")
    .then((result) => {
      console.log("200 res: ", result);
    })
    .catch((e) => {
      console.log("200 e: ", e);
    });

  axios
    .get("https://httpstat.us/429/cors")
    .then((result) => {
      console.log("429 res: ", result);
    })
    .catch((e) => {
      console.log("429 e: ", e);
    });

A PR for this would be to add status code 429 to isRetryableError() then either a new delay function or adding support to the existing ones. Any particular preference?

[parsley72]

Demo of PR here: https://codesandbox.io/s/add-retry-after-support-to-axios-retry-with-cancel-7pgy0

[watsonbox]

In case it's useful I recently implemented a rate limiting strategy using Bottleneck as part of this PR, having come across this while I was looking at possible approaches.

It might seem a little heavyweight as a solution but it's theoretically compatible with any promise-based client, and was really very straightforward to put in place.

[pauldraper]

@parsley72 I think you forgot to convert from seconds to milliseconds. (Also it doesn't handle the datetime version, FYI.)

[ctranstrum]

Note that the npm package retry-axios handles this by default. If this package maintainer doesn't want to add it, it's an easy switch.