feat: use safe-timers to support large delay #19
Conversation
|
@atian25, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fengmk2, @popomore and @dead-horse to be potential reviewers. |
agent.js
Outdated
| @@ -3,6 +3,7 @@ | |||
| const loadSchedule = require('./lib/load_schedule'); | |||
| const parser = require('cron-parser'); | |||
| const ms = require('humanize-ms'); | |||
| const safetimers = require('safe-timers'); | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
昨天本来想自己写个的，结果发现这个了。
很少人会触到这个阈值吧。
agent.js
Outdated
| @@ -98,7 +99,7 @@ function startCron(interval, listener) { | |||
| nextTick = interval.next().getTime(); | |||
| } while (now >= nextTick); | |||
|
|
|||
| setTimeout(() => { | |||
| safetimers.setTimeout(() => { | |||
There was a problem hiding this comment.
只有超过 Math.pow(2, 31) - 1 才使用 safetimers 吧，没超过就使用默认的，这样比较靠谱。
|
有个测试不太稳定，在 6 下挂了 |
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
=========================================
Coverage ? 98.21%
=========================================
Files ? 3
Lines ? 112
Branches ? 0
=========================================
Hits ? 110
Misses ? 2
Partials ? 0
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
=========================================
Coverage ? 98.21%
=========================================
Files ? 3
Lines ? 112
Branches ? 0
=========================================
Hits ? 110
Misses ? 2
Partials ? 0
Continue to review full report at Codecov.
|
|
ci 过了，覆盖率那个是因为 MAX_TIME 没法覆盖。 cc @fengmk2 |
|
需要等 safeTimer 的 PR 先合并么？ |
|
那边有 review 需要 @fengmk2 再改改。 感觉不用等吧，那个 PR 是 cov 和 setInterval 的额外参数的，在这个 PR 里面没用到。 |
|
2.4.0 |
| @@ -3,7 +3,8 @@ | |||
| const loadSchedule = require('./lib/load_schedule'); | |||
| const parser = require('cron-parser'); | |||
| const ms = require('humanize-ms'); | |||
|
|
|||
| const safetimers = require('safe-timers'); | |||
| const MAX_SAFE_TIME = Math.pow(2, 31) - 1; | |||
There was a problem hiding this comment.
呃，明明 safetimers 有暴露 maxInterval 出来 https://github.com/Wizcorp/safe-timers/blob/master/index.js ，直接用它的就好
Checklist
npm testpassesAffected core subsystem(s)
Description of change
引入 https://github.com/Wizcorp/safe-timers