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

Aync forEach handler swallows exceptions #11163

Closed
Roam-Cooper opened this issue Sep 27, 2016 · 6 comments
Closed

Aync forEach handler swallows exceptions #11163

Roam-Cooper opened this issue Sep 27, 2016 · 6 comments

Comments

@Roam-Cooper
Copy link

TypeScript Version: 1.8.10

Code

[1].forEach(async (i)=>{throw 'oh noes';});

Expected behavior:
Exception is propagated outside of the forEach.

Actual behavior:
Exception is swallowed by the forEach and there is no way to capture or handle it (as far as I know).
In my use case code similar to the above is wrapped in an async function and I expected the exception to be propagated to the code I have that handles such things.

@blakeembrey
Copy link
Contributor

@Roam-Cooper This isn't how asynchronous code works in JavaScript or how async works according the specification. async functions return a promise, you need to .catch that promise to handle errors. If you really wanted to catch an error, you could do:

async function outer () {
  await Promise.all([1].map(async (i) => throw 'oh noes'))
} 

@Roam-Cooper
Copy link
Author

Roam-Cooper commented Sep 27, 2016

I know how asynchronous code works in Javascript...

And using Promise.all was going to be my solution if a resounding "NO" was the answer to my request, as it frequently is on this repo.

Your example doesn't address the problem of not being able to interact with the promise used in an asynchronous forEach handler at all. If this is considered a non-issue then I know I can just use Promise.all, if so then it seems the point of allowing forEach to be asynchronous is only to allow asynchronous calls within the handler but with the critical drawback of losing the ability to handle any sort of rejection within the asynchronous forEach handler.

@Meligy
Copy link

Meligy commented Sep 27, 2016

I second Blake. I think it's by design.

Think of this:

[1].forEach(async (i)=>{return i;});

In this case, you also have no way to interact with the return, because forEach does not give the return of the function.

There's another function for that, map.

[1].map(async (i)=>{throw 'oh noes';});

This will give you all the promises returned.

The forEach with async function is a fire-and-forget pattern, which is 99% of the time the wrong thing to do. Same with void async methods in C#, which you only use for event handlers that have defined void return signature, but other that, async methods should return Task. It's the same here with Promise instead of Task.

@Roam-Cooper
Copy link
Author

Roam-Cooper commented Sep 27, 2016

I can see what you mean, but I still think that "getting a returned value" is very different to "catching a thrown exception" inside the forEach loop, because an exception inside a synchronous forEach loop would be correctly thrown which means the behaviour is the polar opposite from synchronous to asynchronous.

Since it seems like people disagree I'll close this issue for now, with the idea that it can be reopened or referenced in the future if the "typescript guys" decide that the behaviour really is odd and I'm not just crazy. 😰

Bubbles an exception:

[1].forEach(i=>{throw i})

Does not bubble an exception:

[1].forEach(async (i)=>{throw i})

@blakeembrey
Copy link
Contributor

blakeembrey commented Sep 27, 2016

@Roam-Cooper That's just how asynchronous code works. This isn't actually specific to TypeScript, it's plain JavaScript. You can try checking out the async/await specification, or a different transpiler like Babel to confirm. They should all act the same, because when you're in an async function throwing makes it a rejected promise. It wouldn't make any sense to have it sometimes throw a synchronous error and sometimes throw an asynchronous error, because there's no such thing as throwing asynchronous errors in JavaScript. Imagine this (no TypeScript involved, we're only talking about JavaScript semantics here):

[].forEach(async (x) => { // (1)
  await wait(500) // (2)
  throw new Error('boom') // (4)
})
// (3)

@blakeembrey
Copy link
Contributor

blakeembrey commented Sep 27, 2016

Have you ever used ES6 generators? Perhaps understanding the runtime behaviour there could help too (http://www.2ality.com/2015/03/es6-generators.html). Feel free to skip if you've never used.

Another way to think about it - async functions transform asynchronous code into synchonrous-looking code. It does this by making the return a Promise object. The same thing applies when you do [1].map(async (i) => { return i }). You aren't going to return i directly because you've marked the function as async - you'll get Promise<i> instead. The same goes for rejections/throw - it becomes a rejected Promise and not a thrown exception like synchronous code.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants