Skip to content

Error handling explanation is not clear #18

Closed
@joshyrobot

Description

@joshyrobot

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()]);

Activity

js-choi

js-choi commented on Jan 3, 2022

@js-choi
Collaborator

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!

added a commit that references this issue on Jan 5, 2022
fa43074
added a commit that references this issue on Jan 6, 2022
0a16ee4
added a commit that references this issue on Jan 6, 2022
18917e4
locked as resolved and limited conversation to collaborators on Jan 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdocumentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @js-choi@joshyrobot

      Issue actions

        Error handling explanation is not clear · Issue #18 · tc39/proposal-array-from-async