Skip to content
This repository was archived by the owner on Jan 3, 2024. It is now read-only.

Conversation

jesec
Copy link
Contributor

@jesec jesec commented May 11, 2021

Bug: #177, vercel/pkg#1163

@jesec jesec requested a review from robertsLando May 11, 2021 13:37
lib/utils.ts Outdated
throw wasReported(e.message);
}
});
return promisify(stream.finished)(ws).then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to await this, also promisify in that way could loose the context, I usually use .bind or .call when using it:

promisify(stream.finished).call(stream, ws)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also could this throw? I see you removed error handling

Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@jesec jesec merged commit 6e5bccd into master May 11, 2021
@jesec jesec deleted the pr/proxy branch May 11, 2021 14:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants