Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Early Returns out of a long promise chain #581

Closed
sidazhang opened this issue Apr 14, 2015 · 14 comments
Closed

Early Returns out of a long promise chain #581

sidazhang opened this issue Apr 14, 2015 · 14 comments

Comments

@sidazhang
Copy link

@sidazhang sidazhang commented Apr 14, 2015

DoSomethingAsync()
.then(step1)
.then(step2)
.then(step3)
.then(step4)
.then(step5)
function step2() {
  if (someCondition) {
    // I want to early return and skip the rest of the promise chain
  }

}

Is there an elegant way to do early return for a long promise chain?

@sidazhang

This comment has been minimized.

Copy link
Author

@sidazhang sidazhang commented Apr 14, 2015

An answer on stackoverflow suggests throwing and error and then catching it.

http://stackoverflow.com/questions/11302271/how-to-properly-abort-a-node-js-promise-chain-using-q

I am wondering if you feel the same or you would suggest an alternative solution

@bergus

This comment has been minimized.

Copy link
Contributor

@bergus bergus commented Apr 15, 2015

@petkaantonov

This comment has been minimized.

Copy link
Owner

@petkaantonov petkaantonov commented Apr 15, 2015

The issue tracker is not for usage questions, stackoverflow or mailing list is better.

@vilic

This comment has been minimized.

Copy link

@vilic vilic commented Nov 24, 2015

@sidazhang I've implemented a fake Promise.break statement to do so. https://github.com/vilic/thenfail

@bergus

This comment has been minimized.

Copy link
Contributor

@bergus bergus commented Nov 24, 2015

@sidazhang You should've called it Promise.goto, and make this .enclose() thing a label :-)

@vilic

This comment has been minimized.

Copy link

@vilic vilic commented Nov 24, 2015

@bergus Actually not a bad idea! I'll think about that, thanks!

@vilic

This comment has been minimized.

Copy link

@vilic vilic commented Jan 2, 2016

@bergus goto and label implemented. ;)

@digital-flowers

This comment has been minimized.

Copy link

@digital-flowers digital-flowers commented May 23, 2016

i would also suggest something like break; in native loop functions or case like switch statement this could be amazing :)

@shanemileham

This comment has been minimized.

Copy link

@shanemileham shanemileham commented Jul 13, 2016

I think it would be a great feature to have a native break and early return - the existing solutions are exceedingly clunky. Can this be opened as a feature request?

@spion

This comment has been minimized.

Copy link
Collaborator

@spion spion commented Jul 13, 2016

For exceptions (unexpected conditions), you can already throw to terminate the chain early. For most other cases, nesting one level works fine. If you have something that doesn't fit the above two solutions feel free to discuss, but given that async/await is just around the corner, it may not be worth the effort (IMO)

@lktvlm

This comment has been minimized.

Copy link

@lktvlm lktvlm commented Jan 14, 2017

You can use asynquence for this, which integrates nicely with bluebird and other promise libraries.
It does a lot of other neat-but-unnecessary stuff, too.
https://davidwalsh.name/asynquence-part-1

@mikermcneil

This comment has been minimized.

Copy link

@mikermcneil mikermcneil commented Jun 22, 2017

@spion Even with async/await, the early return thing comes up. It's to a lesser degree, of course, but it's pretty confusing IMO. I've only recently gotten it straight myself, so I imagine folks new to JavaScript will have a bit of a hard time.

Consider:

// The problem:
(async function(inputs, exits) {

  var result = await doSomething()
  .catch('missing', () => {
    return exits.notFound();
    // ^^^^Note: This could just as easily be `return res.sendStatus(404);`
    // Or `return res.render()` / `return res.redirect()` / etc.
  })
  .catch((e) => {
    return exits.error(e); 
  });

  // THIS WOULD NOT WORK AS EXPECTED BECAUSE, NO MATTER WHAT,
  // THE FOLLOWING CODE WOULD RUN.  ALTERNATIVELY, WE COULD THROW
  // FROM INSIDE THE CATCH HANDLERS, BUT THAT WOULD PREVENT US FROM
  // PROPERLY CALLING EXITS.NOTFOUND(), SINCE IT WOULD JUST HIT THE CATCHALL.

  console.log('Success!!', result);
  return exits.success(); // could just as easily be `return res.json(...);`

})({}, {
  error: function(err) {
    console.error('Hit proper error outlet', err);
  },
  notFound: function() {
    console.error('Hit proper notFound outlet');
  },
  success: function(result) {
    console.error('Hit proper success outlet', result);
  }
}).catch((e) => {
  console.error('Hit unhandled rejection catchall', e);
});

It's totally solvable in the normal, synchronous way:

var result;
try {
  result = await doSomething();
} catch (e) {
  if (e.code === 'notFound') {
    return exits.notFound();
  }
  return exits.error(e);
}

console.log('Success!!', result);
return exits.success();

But it's not necessarily obvious that one should do that... (Especially since everyone is still in the mindset of the pre async/await world)

More thoughts on that:
https://gist.github.com/mikermcneil/d781573e2e30d6963b5586ad69663979

@mikermcneil

This comment has been minimized.

Copy link

@mikermcneil mikermcneil commented Jun 22, 2017

Could maybe even do something like this:

var result = await doSomething()
.catchAndRelease('missing', () => { return exits.notFound(); })
.catchAndRelease((e) => { return exits.error(e); });

// If we were to `throw` in the callback for `.catchAndRelease()`, then that would still
// cause cause the promise to be rejected.  But otherwise, as in this example, the promise
// is unaffected by what you do in that function -- it would not resolve just because you return.
// This would allow us to use it to achieve the same affect as returning early from within
// a catch block in synchronous code.

//... which would solve the problem I believe.
// But that begs the question: would that actually be ok?  Or would that cause a memory leak?
// (would need to experiment / look into how `await` is actually implemented a bit more)

console.log('Success!!', result);
return exits.success();
@challenger532

This comment has been minimized.

Copy link

@challenger532 challenger532 commented Jun 16, 2018

some thing like that will be great:

function step2() {
  if (someCondition) {
     return Promise.neglect()
  }

}

and then neither then nor catch will be called after that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.