Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry is not working when there is an error "TimeoutError: retry timed out" #23

Open
Kushi171987 opened this issue May 21, 2020 · 8 comments

Comments

@Kushi171987
Copy link

I used "the retry-as-promised" in my Sequelize node js server to connect the database.
I added match as
match: [ Sequelize.ConnectionError, Sequelize.ConnectionRefusedError, Sequelize.ConnectionTimedOutError, Sequelize.OptimisticLockError, Sequelize.TimeoutError, "TimeoutError", "retryTimeout", "TimeoutError: retry timed out", // 'SequelizeDatabaseError: Deadlock found when trying to get lock; try restarting transaction', // 'ER_LOCK_DEADLOCK', // 'SQL_BUSY', /SequelizeConnectionError/, /SequelizeConnectionRefusedError/, /SequelizeConnectionTimedOutError/, /SequelizeHostNotFoundError/, /SequelizeHostNotReachableError/, /SequelizeInvalidConnectionError/, /TimeoutError/ ]

though I added the "TimeoutError", its not retrying when happened with that error.

@mickhansen
Copy link
Owner

The timeout error generated by the library is outside the retry logic loop: https://github.com/mickhansen/retry-as-promised/blob/master/index.js#L66

@Kushi171987
Copy link
Author

Kushi171987 commented May 21, 2020

Thank you for the quick response.

I got it now.
To fix the problem I have an option to increase the value of timeout param, but query execution time depends on network traffic & server load.

So is there any option to retry that kind of error?

@mickhansen
Copy link
Owner

Not at this time, needs to be fixed.

@blipk
Copy link

blipk commented Sep 3, 2024

The timeout error generated by the library is outside the retry logic loop: https://github.com/mickhansen/retry-as-promised/blob/master/index.js#L66

@mickhansen

Does this not completely break the purpose?
If the timeout logic is outside of the retry logic then there will never be a retry if the timeout is hit?

Am I missing something?

Attempting to use this in sequelize and this fails without retrying:

    const retryOptions: RetryOptions = {
        match           : [ TimeoutError ],
        report          : console.log,
        max             : 5,
        timeout         : 1 * 1000, // 1 second timeout
        backoffBase     : 10 * 1000,
        backoffExponent : 1
    }
    const options: QueryOptions = { retry: retryOptions }

//takes ~5 seconds on my CPU
    await sequelize.queryRaw( `WITH RECURSIVE r(i) AS (
  VALUES(0)
  UNION ALL
  SELECT i FROM r
  LIMIT 10000000
)
SELECT i FROM r WHERE i = 1;`, options )

@mickhansen
Copy link
Owner

@blipk Correct, as the code is written now, if the timeout is hit it does not trigger retries. I can't recall if that was intended or not, but in retrospect it does not sound ideal.

@blipk
Copy link

blipk commented Sep 4, 2024

Thanks for the quick response and confirmation @mickhansen

Are you intending to work on this?

Would you accept a PR? Any preference on how you would want it implemented?

@mickhansen
Copy link
Owner

Thanks for the quick response and confirmation @mickhansen

Are you intending to work on this?

Would you accept a PR? Any preference on how you would want it implemented?

@blipk I imagine it would be as simple as converting the current setup to use Promise.race instead. The code is generally a bit ancient at this point, I will happily accept PRs - but better libraries may exist :)

@blipk
Copy link

blipk commented Sep 4, 2024

Thanks @mickhansen

My concern is that Sequelize uses this library for its underlying retry logic

I've raised an issue there and will see what they say - sequelize/sequelize#17459

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants