Skip to content
New issue

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

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

Already on GitHub? # to your account

Allow onFailedAttempt to be async #30

Merged
merged 7 commits into from
Oct 31, 2019
Merged

Allow onFailedAttempt to be async #30

merged 7 commits into from
Oct 31, 2019

Conversation

achingbrain
Copy link
Contributor

Builds on #29 and adds tests & docs. Also wraps the invocation of onFailedAttempt in a try/catch to avoid unhandledPromiseRejections if it throws.

Fixes #27.

Kikobeats and others added 2 commits September 25, 2019 21:40
closes #27
Also aborts all retries if `onFailedAttempt` throws.
@achingbrain
Copy link
Contributor Author

@sindresorhus any chance this could be merged?

@sindresorhus sindresorhus changed the title Allow onFailedAttempt to be async Allow onFailedAttempt to be async Oct 30, 2019
readme.md Outdated
},
```

If `onFailedAttempt` throws, all retries will be aborted.
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

If onFailedAttempt throws, all retries will be aborted.

This needs to mention that the original promise will reject with this error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the docs but I'm not sure what changes to make to the .d.ts file, I don't really know any TypeScript, sorry.

Copy link
Owner

Choose a reason for hiding this comment

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

Just add the docs changes to the .d.ts file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the change. Hopefully it's in the right place.

@sindresorhus sindresorhus merged commit 67afe2a into sindresorhus:master Oct 31, 2019
@sindresorhus sindresorhus mentioned this pull request Oct 31, 2019
@achingbrain achingbrain deleted the allow-on-failed-attempt-to-be-async branch October 31, 2019 11:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow onFailedAttempt to be an async function and wait for it to resolve
3 participants