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

feat: exponential backoff #355

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Nov 11, 2021

  • Adds an exponential backoff rather than just a random delay, which I think is more likely to avoid errors.
  • Increases default retries to 5, so that the default maximum retry delay will be 30s (starting at 2s).
  • Adds test that retries terminate.
  • Partially address painful bug I bumped into trying to write tests.

@@ -53,16 +53,23 @@ export class Queue extends EventEmitter {
}
// grab the element at the front of the array
const item = this.q.shift()!;
// Depending on CPU load and other factors setTimeout() is not guranteed to run exactly
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setTimeout's timing is not guaranteed, I was bumping into an issue in tests where it executed a few moments before item.timeToRun, making it so tests never executed.

A better fix would be to have a long running setInterval which ensures the queue is eventually drained. This felt like a more significant change that should happen outside of this PR.

@JustinBeckwith JustinBeckwith merged commit 3f24ea6 into JustinBeckwith:main Nov 11, 2021
@github-actions
Copy link

🎉 This PR is included in version 2.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants