fix: should not throw error when pass null #99
Conversation
| @@ -80,6 +80,9 @@ class Ready extends EventEmitter { | |||
| opt.name = name || cacheKey; | |||
| const timer = setTimeout(() => this.emit('ready_timeout', opt.name), opt.timeout); | |||
| const cb = once(err => { | |||
| if (err != null && !(err instanceof Error)) { | |||
| err = new Error(err); | |||
There was a problem hiding this comment.
Once err is undefined, this will be broken.
Should be err !== null && err !== undefined instead.
There was a problem hiding this comment.
err != null is equal to err !== null && err !== undefined
There was a problem hiding this comment.
How about moving this logic to readyDone? So you can deal with error together at one place.
There was a problem hiding this comment.
Because of the stack, so this way is better rather than after setImmdiate
Codecov Report
@@ Coverage Diff @@
## master #99 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 57 57
=====================================
Hits 57 57
Continue to review full report at Codecov.
|
|
U r right. LGTM
发自我的 iPad
… 在 2017年2月13日，上午12:06，Haoliang Gao ***@***.***> 写道：
@popomore commented on this pull request.
In lib/ready.js:
> @@ -80,6 +80,9 @@ class Ready extends EventEmitter {
opt.name = name || cacheKey;
const timer = setTimeout(() => this.emit('ready_timeout', opt.name), opt.timeout);
const cb = once(err => {
+ if (err != null && !(err instanceof Error)) {
+ err = new Error(err);
Because of the stack, so this way is better rather than after setImmdiate
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Going to merge and I will release a patch version. |
|
2.0.1 |
No description provided.