Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: less async await #2464

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class Agent extends DispatcherBase {
return dispatcher.dispatch(opts, handler)
}

async [kClose] () {
[kClose] () {
const closePromises = []
for (const ref of this[kClients].values()) {
const client = ref.deref()
Expand All @@ -128,10 +128,10 @@ class Agent extends DispatcherBase {
}
}

await Promise.all(closePromises)
return Promise.all(closePromises)
}

async [kDestroy] (err) {
[kDestroy] (err) {
const destroyPromises = []
for (const ref of this[kClients].values()) {
const client = ref.deref()
Expand All @@ -141,7 +141,7 @@ class Agent extends DispatcherBase {
}
}

await Promise.all(destroyPromises)
return Promise.all(destroyPromises)
}
}

Expand Down
8 changes: 5 additions & 3 deletions lib/api/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,14 @@ function isUnusable (self) {
return util.isDisturbed(self) || isLocked(self)
}

async function consume (stream, type) {
function consume (stream, type) {
if (isUnusable(stream)) {
throw new TypeError('unusable')
return Promise.reject(new TypeError('unusable'))
}

assert(!stream[kConsume])
if (stream[kConsume]) {
return Promise.reject(new assert.AssertionError('null != true'))
}

return new Promise((resolve, reject) => {
stream[kConsume] = {
Expand Down
4 changes: 2 additions & 2 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ class Client extends DispatcherBase {
return this[kNeedDrain] < 2
}

async [kClose] () {
[kClose] () {
// TODO: for H2 we need to gracefully flush the remaining enqueued
// request and close each stream.
return new Promise((resolve) => {
Expand All @@ -392,7 +392,7 @@ class Client extends DispatcherBase {
})
}

async [kDestroy] (err) {
[kDestroy] (err) {
return new Promise((resolve) => {
const requests = this[kQueue].splice(this[kPendingIdx])
for (let i = 0; i < requests.length; i++) {
Expand Down
4 changes: 2 additions & 2 deletions lib/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ function extractBody (object, keepalive = false) {
}
return controller.desiredSize > 0
},
async cancel (reason) {
await iterator.return()
cancel (reason) {
return iterator.return()
Comment on lines +237 to +238
Copy link
Member

Choose a reason for hiding this comment

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

This might hide it from the stacktrace, not sure the attempt is to keep that function hidden as is part of the RedeableStream API and can be helpful while debugging.

},
type: undefined
})
Expand Down
8 changes: 4 additions & 4 deletions lib/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1807,11 +1807,11 @@ async function httpNetworkFetch (
async start (controller) {
fetchParams.controller.controller = controller
},
async pull (controller) {
await pullAlgorithm(controller)
pull (controller) {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

the streams spec will wrap this in a promise, so it does nothing

return pullAlgorithm(controller)
},
async cancel (reason) {
await cancelAlgorithm(reason)
cancel (reason) {
return cancelAlgorithm(reason)
}
},
{
Expand Down
4 changes: 2 additions & 2 deletions lib/pool-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class PoolBase extends DispatcherBase {
return this[kStats]
}

async [kClose] () {
[kClose] () {
if (this[kQueue].isEmpty()) {
return Promise.all(this[kClients].map(c => c.close()))
} else {
Expand All @@ -121,7 +121,7 @@ class PoolBase extends DispatcherBase {
}
}

async [kDestroy] (err) {
[kDestroy] (err) {
while (true) {
const item = this[kQueue].shift()
if (!item) {
Expand Down
66 changes: 31 additions & 35 deletions lib/proxy-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,40 +82,38 @@ class ProxyAgent extends DispatcherBase {
this[kClient] = clientFactory(resolvedUrl, { connect })
this[kAgent] = new Agent({
...opts,
connect: async (opts, callback) => {
connect: (opts, callback) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is unnecessary change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh. This is for discussing. I agree this is reducing the maintainability. I am unsure if we save one Promise by this.

let requestedHost = opts.host
if (!opts.port) {
requestedHost += `:${defaultProtocolPort(opts.protocol)}`
}
try {
const { socket, statusCode } = await this[kClient].connect({
origin,
port,
path: requestedHost,
signal: opts.signal,
headers: {
...this[kProxyHeaders],
host
}
})
if (statusCode !== 200) {
socket.on('error', () => {}).destroy()
callback(new RequestAbortedError('Proxy response !== 200 when HTTP Tunneling'))
}
if (opts.protocol !== 'https:') {
callback(null, socket)
return
this[kClient].connect({
origin,
port,
path: requestedHost,
signal: opts.signal,
headers: {
...this[kProxyHeaders],
host
}
let servername
if (this[kRequestTls]) {
servername = this[kRequestTls].servername
} else {
servername = opts.servername
}
this[kConnectEndpoint]({ ...opts, servername, httpSocket: socket }, callback)
} catch (err) {
callback(err)
}
}).catch(callback)
.then(({ socket, statusCode }) => {
if (statusCode !== 200) {
socket.on('error', () => { }).destroy()
callback(new RequestAbortedError('Proxy response !== 200 when HTTP Tunneling'))
}
if (opts.protocol !== 'https:') {
callback(null, socket)
return
}
let servername
if (this[kRequestTls]) {
servername = this[kRequestTls].servername
} else {
servername = opts.servername
}
this[kConnectEndpoint]({ ...opts, servername, httpSocket: socket }, callback)
}).catch(callback)
}
})
}
Expand All @@ -136,14 +134,12 @@ class ProxyAgent extends DispatcherBase {
)
}

async [kClose] () {
await this[kAgent].close()
await this[kClient].close()
[kClose] () {
return Promise.all([this[kAgent].close, this[kClient].close])
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved
}

async [kDestroy] () {
await this[kAgent].destroy()
await this[kClient].destroy()
[kDestroy] () {
return Promise.all([this[kAgent].destroy, this[kClient].destroy])
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Loading