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

fix: ensure node retries happen when using .then syntax (#1487) #1543

Merged

Conversation

charlenetshos
Copy link
Contributor

@charlenetshos charlenetshos commented Feb 27, 2020

This PR is to enforce consistent retry behaviour for both the.end and .then syntaxes.

The problem I'm trying to fix has been highlighted in these issues:

When using .then syntax in Node.js, we only want to abort a request if there are no retries or the maximum number of retries has been exceeded.

Please review

@charlenetshos
Copy link
Contributor Author

Hello @niftylettuce! We're quite keen to see this fixed as we're having a number of socket timeout errors. I'm wondering when you'll have an opportunity to review this PR. Is there something I can do to make it happen? Thank you

@niftylettuce niftylettuce merged commit 77bcb11 into ladjs:master Jun 28, 2020
@niftylettuce
Copy link
Collaborator

v5.3.0 published to npm and GitHub releases page with changelog

@charlenetshos charlenetshos deleted the fix/ensure_retry_on_timeout branch July 16, 2020 10:22
@charlenetshos
Copy link
Contributor Author

@niftylettuce thank you for releasing this!

@niftylettuce
Copy link
Collaborator

no prob

@niftylettuce
Copy link
Collaborator

niftylettuce commented Aug 8, 2020

Just so you're aware, I had to revert this because the tests failed.

@niftylettuce
Copy link
Collaborator

Oh sorry, I commented on the wrong issue. Ignore this.

@niftylettuce
Copy link
Collaborator

For reference, or if you want to help debug, see #1527.

This was referenced Jul 16, 2024
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

Successfully merging this pull request may close these issues.

2 participants