Error handling questions

[benjamingr]

Hey a few questions to make sure:

• if an error happens when a .map(fn) throws, the error propagates to the consumer, right? (i.e. .next returns a rejected promise and the iterator is closed).
• if an error happens in an argument, does it happen immediately or when starting the iterator? (that is: does .map(1) throw synchronously or when the generator is first used?)

[devsnek]

the methods in this spec are built kind of like this:

function map(f) {
  if (typeof f !== 'function') throw TypeError;

  return (function* (){
    ...
  })();
}

so the errors for the arguments are immediate, and the errors during iteration are handled more or less like a normal generator would do

[ljharb]

That sounds right for sync methods; but I’d expect the async methods to never throw synchronously.

[devsnek]

async generators don't return a promise to the generator instance, they return the instance. there's not really a chance to reject anything there.

[ljharb]

Awesome, then they should throw on bad arguments too :-)

[benjamingr]

@devsnek correct, so can you confirm they should throw synchronously on bad arguments rather than put the generator in a state where consuming it (.next) throws the error for the argument?

[devsnek]

that matches my interpretation of the current spec

[benjamingr]

Great thanks.

[benjamingr]

Bug to fix in Node for future reference: nodejs/node#41648

[Jack-Works]

When an error happens, it won't call .throw() on the original iterator. Is that intentional? (because spec is using IfAbruptCloseIterator)

[bakkot]

@Jack-Works That seems correct. Consider the analogy of for of: if you have

for (let item of iterable) {
  throw 0;
}

that will call .return, not .throw, on the iterator.

[hax]

@bakkot I think for-of is different, for-of is just a specific usage, never passing protocol, so it's ok to not invoke upstream throw(). I think the analogy should be:

function *g() {
  yield *iter
}

And g.throw() actually invoke iter.throw().

I think people would expect yield *iter.map(x => x) have exact same behavior with yield *iter.

Anyway, it seems the subset of the passing protocol issue #122 .

[bakkot]

@hax Because of how throw works, there's not really a difference between ".throw was called on x.map(f)" and "f threw in x.map(f)". And the latter definitely shouldn't invoke .throw on x. So the former shouldn't either.

Right now nothing in the spec invokes .throw unless you explicitly call it at some point; I don't think we should change that.

I don't think you should think of yield* iter.map(x => x) as behaving like yield* iter. I think you should think of it as behaving like for (let item of iter) yield item. That's the only thing which makes sense when the mapping function is more complex than x => x.

[benjamingr]

I agree, the way I see it .throw is and was designed to imitate throwing "in the other direction" on the generator so it would be a way to signal to the iteration that it has done something to result in a bad state. This is all pretty philosophical.

When you have iter.map(someFunction) and someFunction throws it indicates someFunction did something wrong and not that the iteration itself is in a bad state. So for example:

function* naturals() {
  for(let i = 0; i < Infinity; i++) yield i;
}
naturals().map(x => { throw new Error(); }).toArray();

In the example - did naturals itself do anything that indicates it did anything faulty that should trigger its error handling logic? I'd argue its cleanup logic should run (.return) so that finally blocks would run and resources would be cleaned up but that its error handling logic (likely catch) should not.

In practice - I think .throw (and .return) were built for a proposal and a future that never came so it's ultimately not super important what this proposal does in this case. We've had a semi-broken implementation of what happens when you .throw in Node.js and a lot of ecosystem libraries make this tradeoff inconsistently and people haven't noticed despite the fact people typically complain (helpfully, quickly and rightfully so) pretty vocally whenever they notice unexpected behavior or issues.

[bakkot]

In practice - I think .throw (and .return) were built for a proposal and a future that never came so it's ultimately not super important what this proposal does in this case

.return I think makes sense as-is. It performs cleanup. It works well and is integrated in the language - in particular, for of calls it when exiting early, so consumers typically get cleanup "for free", without having to know about .return at all.

But yeah, .throw doesn't make so much sense. My uninformed speculation is that it's a legacy inherited from Python, which uses exceptions for control flow in a way idiomatic JS very much does not. And since nothing in the language calls it (without you having called it first), it has ended up not really integrated with the language.

[benjamingr]

@bakkot the history was when Jafar worked on the protocol there was an attempt to make it "dual" to observables for that proposal (now dead and changed) and compositional functions (also dead).

Consider an alternate universe where:

• async functions never landed in the language
• instead, decorators did
• decorators can now de-sugar async functions

In this alternate timeline it is important that generators can communicate .throw in that direction. The discussion with Erik there was interesting at least.

Honestly it is one of those cases I'm happy we didn't go that route since it was quite confusing. In any case - Python did not play a part afaik.

As for why .return is used both for termination/cleanup and explicit disposal I have an open-"mark as unread" on #164 (comment) for a year now and I'm still thinking about it :D I will reference Jafar's thoughts when he worked on it and hopefully they can shed some light on the topic tc39/proposal-observable#33 (comment)