Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

What happen when next() is called on a cleanup iterator outside the finalization turn? #29

Closed
dtribble opened this issue Apr 11, 2018 · 6 comments · Fixed by #84
Closed

Comments

@dtribble
Copy link
Contributor

dtribble commented Apr 11, 2018

The spec says that the iterator does not produce, so what does it do? Options

  • throw
  • return "done"
  • something else

(from @bmeck)

@erights
Copy link
Contributor

erights commented Apr 11, 2018

I think I prefer "throw" though I see arguments for "done". Either seem like something we can live with. What other options might there be?

@littledan
Copy link
Member

The current specification text doesn't have a clear answer here. There's logic to return "done" when [[FinalizationGroup]] becomes undefined, but I don't see anything that sets it that way. I don't have an opinion between throw and return "done". It seems like it'd be pretty easy to set the FinalizationGroup to undefined when the finalization turn is done.

@mhofman
Copy link
Member

mhofman commented Apr 25, 2019

As I mentioned in #84, is it invalid to have cleanupCallback be an async function?

@littledan
Copy link
Member

In general, code shouldn't be distinguishing between sync and async functions, just treating all functions which return a Promise the same. In this case, no one cares about the return value of this function. But if it's async and awaits, it may cause the exception to be thrown.

@mhofman
Copy link
Member

mhofman commented Apr 30, 2019

An exception thrown is exactly what I'm concerned about, especially because the exception would only be thrown if the iterator yields more than one element.

As a developer, I might be surprised to have code like this fail:

finalizationGroup.cleanupSome(async (items) => {
  for (const holding of items) {
    await freeStuff(holding);
  }
});

@gsathya
Copy link
Member

gsathya commented Apr 30, 2019

I think it's fine to throw an exception in that case. A good error message should make it clear what the problem is and how to fix it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants