Skip to content

Conversation

@uniqueiniquity
Copy link
Contributor

@uniqueiniquity uniqueiniquity commented Oct 5, 2018

Fixes #27544

Currently, if a promise handler that is itself returning a promise is changed to an async function using our code fix, that promise is not awaited if it will be in a return position in the resulting async function. This was done since the following are equivalent in evaluation:

async function f() {
  return Promise.resolve(3);
}

async function g() {
  return await Promise.resolve(3);
}

However, if a rejected promise is returned from within a try block, awaiting the promise will cause it to be caught by the catch block, whereas returning it without an await will pass the rejected promise to the caller.

This PR changes the code fix behavior so promises returned by promise handlers will always be awaited, regardless of whether they are in a return position or not. This does mean that "unnecessary" awaits will be added, but for promises that might potentially be rejected but are, at the time of the code fix, not caught, the extra await may avoid later issues if wrapped in a catch after the fact.

Thanks @bterlson for input!

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

codefix convertToAsync does not add await for returned values

2 participants