Doesn't retry on timeout

[antony]

I think the title explains it. The code specifies

error.code !== 'ECONNABORTED'

and there is no way to override it.

Retrying when an endpoint timeouts is pretty much our only use for a retry module. I'd like to be able to override all the retry conditions - but I'm also confused as to why we wouldn't retry on a timeout in the first place?

[erdii]

I think this is because of #5

[antony]

Thanks @erdii - that issue doesn't make any sense at all. Timeouts should definitely be retried. For us at least, it's the only reason we want to retry!

[erdii]

I'm testing the lib right now and it seems that it doesn't retry if you disconnect from the network (on mobile) at all.

maybe we should give tomaash/axios-status a try. (even though it changes the request interface)

[antony]

I've had to revert back to got right now because this transition is taking too long. next time I venture into it I will look at axios-status so thanks for that!

[rubennorte]

The error that's being specifically ignored in the code is ECONNABORTED, which means that the program itself cancelled the request due to a specific cancellation or a timeout. What use case do you have that you need to retry those? Why don't you simply increment the timeout?

Retrying on timeout could make requests too slow, as if something goes wrong you'll be paying the cost of the timeout for each call.

[antony]

What we do is talk to a variety of different bank APIs to get loans. Some of these APIs can take upto 2 minutes to reply, and often they will take longer. What we do is set our timeout to 2 minutes, and if they take longer, we just retry the request. We do this up to 5 times before giving up.

With got (native)/request (requestretry) this is no problem, but I'd like to switch to axios - however this ECONNABORTED check prevents me from doing so.

[Lxxyx]

Add a new config? retryOnTimeout, set default to false.

In someway, timeout means I wants to cancel this request and retry.

[erdii]

A new option retryCondition (Default: error => !error.response) has been added.
This issue can be closed now, am I right?

Thank you @rubennorte!

Edit: I still cannot use the retryCondition function to retry ECONNABORTED requests because of this guy here:

const shouldRetry = retryCondition(error)
      && error.code !== 'ECONNABORTED'
      && config.retryCount < retries
      && isRetryAllowed(error);

shouldn't this be:

const shouldRetry = retryCondition(error)
      && config.retryCount < retries;

and the default retryCondition something like error => !error.response && isRetryAllowed(error) && error.code !== 'ECONNABORTED'?

EDIT2: add forgotten ECONNABORTED handling
EDIT3: I'd happily create a pull request containing the code changes and tests if you approve this change

[Lxxyx]

@erdii idea is great. Can you change the code? @antony

[antony]

Yes @erdii - this is exactly what I was suggesting. To be able to configure the retry conditions in this way is absolutely critical.

[erdii]

I'm a bit busy right now - but I have school next week so I'll have plenty of time to implement it and create a pull request. Is this ok? @Lxxyx @antony ?

@rubennorte are you willing to accept my pr when i provide test coverage and doc updates? :)

[erdii]

I have implemented the changes, tests, and documentation at erdii@25faf8f. Creating a pull request now.

[rubennorte]

We're going to make some changes in the library soon, including one to solve this issue. What do you think about:

import axios from 'axios';
import axiosRetry from 'axios-retry';

axiosRetry(axios, {
  retryCondition: axiosRetry.isNetworkError, // default
  retryCondition: axiosRetry.isAnyError // alternative
  retryCondition: (response) => {  // custom
    return axiosRetry.isNetworkError(response) || response.config.method === 'GET'
  }
});

[antony]

Seems reasonable to me @rubennorte. I'd also recommend adding some sort of retryDelay parameter which can be a simple number, or a function which is passed the last error, and the current retry count, allowing for exponential decays, thus:

// with retries: 7, retries 7 times with a 1 second gap.
retryDelay: 1000

// for timeouts, wait longer before retrying. For all other errors, just 1 second.
retryDelay: (error) => { return (error.message === 'ETIMEDOUT') ? 60000 : 1000 }

// linear decay
retryDelay: (error, count) => { return count * 1000 }

// exponential decay
retryDelay: (error, count) => { return (count * count) * 1000 }

Maybe this is a new feature rather than an addition to this one, but it's something got has built in - and ultimately was our reason for switching to it. We really want our payloads to go through, and our providers (banks!) are super-flaky sometimes.

[raychenfj]

Hi @rubennorte,

I check you latest version, but this line still prevent it retry on timeout.

error.code !== 'ECONNABORTED'

I think put it into the default retryCondition is a better idea, so anyone doesn't need it can override it. I can send a PR if you need.

export function isNetworkError(error) {
  return !error.response && error.code !== 'ECONNABORTED';
}

Thanks for making this lib, I use it in my projects, but I have to make a copy and remove that line every time., it's really inconvenient.

[jony89]

@rubennorte

Still I can't use the lib implementation overall.

isNetworkError, isSafeRequestError and isIdempotentRequestError still checks for :

error.code !== 'ECONNABORTED'

This error code received when defining axios a timeout, and it has passed.
This is like the most wanted scenario to retry.

So currently I still cant use this function implementation.

Maybe you could have a configuration option to decide whether or not to check for ECONNABORTED ?

[rubennorte]

As stated in a PR that was adding that as a configuration (which was rejected as we're trying to keep the API minimal), you can still retry timeout errors using the built-in functions with way:

retryCondition: (error) => {
  return axiosRetry.isNetworkOrIdempotentRequestError(error)
    || error.code === 'ECONNABORTED';
}

[tatianajiselle]

Weird, ive implemented the override via retryCondition() with return true and my retries dont trigger.

---

RESOLVED:

full config.. im using nestjs so access to axios is through the httpService reference and configuring a http agent timeout...

After some debugging I realized that for this to work, shouldRetryOnTimeout need to be set to true, otherwise the timeout which is inherited by the http module agent configuration ends up timing out and the retry condition never triggers.

hopefully this will help someone else!

axiosRetry(httpService.axiosRef, {
            retries: 3,
            retryDelay: () => 100, // in milliseconds
            shouldResetTimeout: true,
            retryCondition: (error: AxiosError) => {
                return (
                    isNetworkOrIdempotentRequestError(error) ||
                    error.code === "ECONNABORTED"
                );
            },
...
}