refactor(dnshttpclient): use async function instead of Promise#2774
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2774 +/- ##
==========================================
+ Coverage 99.39% 99.75% +0.36%
==========================================
Files 29 29
Lines 824 825 +1
==========================================
+ Hits 819 823 +4
+ Misses 5 2 -3
Continue to review full report at Codecov.
|
|
rebase 一下 |
| .then(result => { | ||
| return super.request(result.url, result.args); | ||
| }) | ||
| .then(result => callback(null, result)) |
There was a problem hiding this comment.
如果 callback 执行抛错，又会进入到下面的 catch 导致二次执行，需要把下面的 catch 放到 then 的第二个参数中
|
|
||
| curl(url, args, callback) { | ||
| if (callback) { | ||
| this.app.deprecate('[dnscache_httpclient] Please don\'t use callback when `curl()`'); |
There was a problem hiding this comment.
这里会输出两条 message 吧，统一在 request() 中提示好了？
| // make sure not callback twice | ||
| callback = null; | ||
| if (record && now - record.timestamp < this.dnsCacheLookupInterval) { | ||
| return { url: formatDnsLookupUrl(hostname, url, record.ip), args }; |
There was a problem hiding this comment.
record.timestamp = now; 的逻辑不需要了？
There was a problem hiding this comment.
原来的逻辑看起来是直接响应，然后异步更新的，是否就改成同步等待？
如果要保留异步实现的话，可能要用到 runInBackground @fengmk2
| .then(result => { | ||
| return super.request(result.url, result.args); | ||
| }) | ||
| .then(result => process.nextTick(() => callback(null, result.data, result.res))) |
There was a problem hiding this comment.
不需要加 nextTick 吧，Promise 应该本来就是异步了，可以直接:
.then(result => callback(null, result.data, result.res), callback)
| if (needUpdate) { | ||
| if (now - record.timestamp >= this.dnsCacheLookupInterval) { | ||
| // make sure next request don't refresh dns query | ||
| record.timestamp = now; |
| */ | ||
| config.httpclient = { | ||
| enableDNSCache: false, | ||
| dnsCacheLookupInterval: 10000, |
|
@fengmk2 @dead-horse 再看看? |
|
@popomore rebase 再跑一遍测试，然后合并。 |
|
@fengmk2 过了 |
|
@dead-horse @fengmk2 没发布前再看看，有问题我再改下 |
3 participants
Checklist
npm testpassesAffected core subsystem(s)
Description of change
and deprecate callback