Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix wrongly called timeouts #5838

Merged
merged 1 commit into from
Jun 19, 2017
Merged

Fix wrongly called timeouts #5838

merged 1 commit into from
Jun 19, 2017

Conversation

ngotchac
Copy link
Contributor

Avoid doing catch(nextTimeout) since some nextTimeout functions have the actual timeout as first argument. Calling this would pass the error as the timeout, which would make it instantaneous:

console.log(Date.now());
setTimeout(() => {
  console.log(Date.now());
}, new Error('This is an error'));

The consequences being sending thousands of requests on continuous failure.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jun 14, 2017
Copy link
Contributor

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

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

So the error passed to catch's callback invalidated the default time of 1000?

Looks good to me.

@maciejhirsz maciejhirsz added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 19, 2017
@ngotchac
Copy link
Contributor Author

@maciejhirsz Yes that's write. If you call setTimeout or setInterval with something else than a Number, it takes 0 as this argument

@maciejhirsz maciejhirsz merged commit b824bc2 into master Jun 19, 2017
@debris debris deleted the ng-fix-timeouts branch June 19, 2017 14:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants