tc39 / proposal-array-from-async

Draft specification for a proposed Array.fromAsync method in JavaScript.
http://tc39.es/proposal-array-from-async/
BSD 3-Clause "New" or "Revised" License
177 stars 13 forks source link

Error handling explanation is not clear #18

Closed joshyrobot closed 2 years ago

joshyrobot commented 2 years ago

I've read #16, and I think I understand and agree with the conclusions that it came to. This is an issue I have with the explanation in the README that was added after closing that issue.

Specifically, this example makes no sense to me:

const err = new Error;
// The creator of the rejecting promise attaches a rejection handler.
const rejection = Promise.reject(err).catch(console.error);
function * genZeroThenRejection () {
  yield 0;
  yield rejection;
}

// This still creates a promise that will reject with `err`. `err` will also
// separately be printed to the console due to the rejection handler.
Array.fromAsync(genZeroThenRejection());

How can .fromAsync ever reject if the function never yields a rejecting promise? The name rejection makes it look like it should reject, but the error was already caught and discarded by console.error.

The surrounding text doesn't help explain much:

Array.fromAsync will not handle any yielded rejecting promises. ... The creator of the rejecting promise is expected to synchronously attach a rejection handler when the promise is created

If the goal is to warn that multiple already-created promises that end up rejecting in the input will lead to an uncaught promise exception, then I think it'd be best to just say that. Because Array.fromAsync will handle the first rejection it sees, if only to pass it up and stop further iteration.

When it comes to Promise.all, I think I understand this part:

Alternatively, the user of the promises can switch from Array.fromAsync to Promise.all. Promise.all would change the control flow from lazy sync iteration (with sequential awaiting) to eager sync iteration (with parallel awaiting), allowing the handling of any rejection in the input.

I interpreted it as "Promise.all might reject with an error from any of the promises, because it eagerly looks ahead". The code sample following it doesn't seem to demonstrate that, though. It has the exact same outcome as the initial Array.fromAsync example (without the .catch(console.error))

const err = new Error;
const rejection = Promise.reject(err);
function * genZeroThenRejection () {
  yield 0;
  yield rejection;
}

// Creates a promise that will reject with `err`. Unlike Array.fromAsync,
// Promise.all will handle the `rejection`.
Promise.all(genZeroThenRejection());
// vs.
// This creates a promise that will reject with `err`. However, `rejection`
// itself will not be handled by Array.fromAsync.
Array.fromAsync(genZeroThenRejection());

Both of these handle rejection, and no uncaught rejection error will happen (excluding the output promises).

As far as I can tell the only actual differences in error handling between the two are that Array.fromAsync always checks each item in order, and will stop on the first rejection (meaning it will not catch any rejections past the first one). I think an explanation like this could suffice:

function waitAndReject(err) {
  return new Promise((resolve, reject) => {
    setTimeout(() => reject(err), 1000);
  });
}

function * genZeroAndRejections () {
  yield 0;
  yield waitAndReject(new Error('delayed'));
  yield Promise.reject(new Error('immediate'));
}

// This will create a promise that rejects immediately with the
// "immediate" error, because Promise.all eagerly awaits all
// yielded items and catches all further errors.
Promise.all(genZeroAndRejections());

// This will create a promise that rejects after 1000ms with the
// "delayed" error, because Array.fromAsync awaits each yielded
// item individually before continuing. The "immediate" error is never
// created because iterations stops early.
Array.fromAsync(genZeroAndRejections());

// This will create a promise that behaves the same, but it will also
// create an Unhandled Promise Rejection error because the promise
// that rejects "immediate" was already created by collecting into an array.
Array.fromAsync([...genZeroAndRejections()]);
js-choi commented 2 years ago

How can .fromAsync ever reject if the function never yields a rejecting promise? The name rejection makes it look like it should reject, but the error was already caught and discarded by console.error.

You are correct. This code block’s comment contains a typo due to copying and pasting. Array.fromAsync will not return a rejecting promise in this case. I will fix this.

Both of these handle rejection, and no uncaught rejection error will happen (excluding the output promises).

This code block also contains typos, which I need to fix. I had forgotten to include a setTimeout-based delay on the zeroth item, as in your own example. I will fix this, too.

Thanks for reporting these errors!