goodeggs / librato-node

A Node.js client for Librato (http://librato.com/)
MIT License
37 stars 26 forks source link

fix: avoid calling setTimeout() with overflowing timeout integer arg #59

Closed serhalp closed 4 years ago

serhalp commented 4 years ago

bc450bf chore: upgrade really old sinon and friends

to get some newer methods on the fake clock interface

7d83f21 refactor: clean up code style a bit

I wanted confidence that I wasn't breaking anything. And I was having trouble understanding the code and this helped.

dc46770 fix: avoid calling setTimeout() with overflowing timeout integer arg

If the clock date moves backwards, say from 2020 to 2010, the librato-node worker schedules a setTimeout() callback for ten years in the future, which causes node to log a warning due to 32-bit signed integer overflow:

(node:10427) TimeoutOverflowWarning: 55619400000 does not fit into a 32-bit signed integer.
Timeout duration was set to 1.

In practice, the behavior isn't great - that callback will basically never fire. We could fix this by updating the logic to handle this somewhat gracefully, but if your clock is moving backwards, you have other issues to deal with anyway. So this just focuses on suppressing the warning by ensuring we don't pass a massive timeout to setTimeout(). This happens in tests where we stub the clock.

I arbitrarily capped it to the period (default 60s). This means the behavior in practice is still not great - it will schedule another check every 60s for eternity. But that seems about the same as before, to me.