Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

docs: Fix Promise error handling examples#680

Merged
MylesBorins merged 1 commit into
nodejs:masterfrom
Oaphi:patch-1
May 5, 2020
Merged

docs: Fix Promise error handling examples#680
MylesBorins merged 1 commit into
nodejs:masterfrom
Oaphi:patch-1

Conversation

@Oaphi

@Oaphi Oaphi commented May 4, 2020

Copy link
Copy Markdown
Contributor

Description

Tweaked function calls in examples in "Exceptions with promises" section to reduce possible confusion vectors:

  1. doSomething1 is a function that returns a Promise in both examples
  2. doSomething2 and doSomething3 should not be called until the doSomething1 promise resolves if they return a Promise or the chain will be broken.
  3. In examples of mid-chain handling, there is an extra parenthesis in first then

This will result in correct order: AA (after 2s) -> BB (after 3s)

new Promise(
    (r, j) => setTimeout(() => {
        console.log("AA");
        r();
    }, 2000)
).then(
    () => new Promise(
        (r, j) => setTimeout(() => {
            console.log("BB");
            r();
        }, 1000)
    )
);

But this won't (as promise is scheduled immediately): BB (after 1s) -> AA (after 2s)

new Promise(
    (r, j) => setTimeout(() => {
        console.log("AA");
        r();
    }, 2000)
).then(
    (() => new Promise(
        (r, j) => setTimeout(() => {
            console.log("BB");
            r();
        }, 1000)
    ))()
)

@Oaphi Oaphi force-pushed the patch-1 branch 2 times, most recently from db6f163 to 3eb4beb Compare May 5, 2020 15:03

@MylesBorins MylesBorins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MylesBorins MylesBorins merged commit d441548 into nodejs:master May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants