403

Suppose I have the following code.

function divide(numerator, denominator) {
 return new Promise((resolve, reject) => {

  if(denominator === 0){
   reject("Cannot divide by 0");
   return; //superfluous?
  }

  resolve(numerator / denominator);

 });
}

If my aim is to use reject to exit early, should I get into the habit of returning immediately afterward as well?

sam
  • 4,033
  • 2
  • 10
  • 4
  • 14
    Yes, due to [run-to-completion](https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop#Run-to-completion) –  Jul 10 '16 at 08:11

6 Answers6

575

The return purpose is to terminate the execution of the function after the rejection, and prevent the execution of the code after it.

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {

    if (denominator === 0) {
      reject("Cannot divide by 0");
      return; // The function execution ends here 
    }

    resolve(numerator / denominator);
  });
}

In this case it prevents the resolve(numerator / denominator); from executing, which is not strictly needed. However, it's still preferable to terminate the execution to prevent a possible trap in the future. In addition, it's a good practice to prevent running code needlessly.

Background

A promise can be in one of 3 states:

  1. pending - initial state. From pending we can move to one of the other states
  2. fulfilled - successful operation
  3. rejected - failed operation

When a promise is fulfilled or rejected, it will stay in this state indefinitely (settled). So, rejecting a fulfilled promise or fulfilling a rejected promise, will have not effect.

This example snippet shows that although the promise was fulfilled after being rejected, it stayed rejected.

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) {
      reject("Cannot divide by 0");
    }

    resolve(numerator / denominator);
  });
}

divide(5,0)
  .then((result) => console.log('result: ', result))
  .catch((error) => console.log('error: ', error));

So why do we need to return?

Although we can't change a settled promise state, rejecting or resolving won't stop the execution of the rest of the function. The function may contain code that will create confusing results. For example:

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) {
      reject("Cannot divide by 0");
    }
    
    console.log('operation succeeded');

    resolve(numerator / denominator);
  });
}

divide(5, 0)
  .then((result) => console.log('result: ', result))
  .catch((error) => console.log('error: ', error));

Even if the function doesn't contain such code right now, this creates a possible future trap. A future refactor might ignore the fact that the code is still executed after the promise is rejected, and will be hard to debug.

Stopping the execution after resolve/reject:

This is standard JS control flow stuff.

  • Return after the resolve / reject:

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) {
      reject("Cannot divide by 0");
      return;
    }

    console.log('operation succeeded');

    resolve(numerator / denominator);
  });
}

divide(5, 0)
  .then((result) => console.log('result: ', result))
  .catch((error) => console.log('error: ', error));
  • Return with the resolve / reject - since the return value of the callback is ignored, we can save a line by returning the reject/resolve statement:

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) {
      return reject("Cannot divide by 0");
    }

    console.log('operation succeeded');

    resolve(numerator / denominator);
  });
}

divide(5, 0)
  .then((result) => console.log('result: ', result))
  .catch((error) => console.log('error: ', error));
  • Using an if/else block:

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) {
      reject("Cannot divide by 0");
    } else {
      console.log('operation succeeded');
      resolve(numerator / denominator);
    }
  });
}

divide(5, 0)
  .then((result) => console.log('result: ', result))
  .catch((error) => console.log('error: ', error));

I prefer to use one of the return options as the code is flatter.

SherylHohman
  • 16,580
  • 17
  • 88
  • 94
Ori Drori
  • 183,571
  • 29
  • 224
  • 209
  • 36
    Worth noting that the code won't actually behave differently if the `return` is there or not because once a promise state has been set, it can't be changed so calling `resolve()` after calling `reject()` won't do anything except use a few extra CPU cycles. I, myself, would use the `return` just from a code cleanliness and efficiency point of view, but it is not required in this specific example. – jfriend00 Sep 12 '15 at 07:21
  • 2
    Try using `Promise.try(() => { })` instead of new Promise and avoid using resolve/reject calls. Instead you could just write `return denominator === 0 ? throw 'Cannot divide by zero' : numerator / denominator;` I use `Promise.try` as a means to kick off a Promise as well as eliminate promises wrapped in try/catch blocks which are problematic. – kingdango Sep 27 '16 at 20:16
  • 2
    It's good to know, and I like the pattern. However, at this time [Promise.try](https://github.com/ljharb/proposal-promise-try) is a stage 0 suggestion, so you can only use it with a [shim](https://www.npmjs.com/package/es6-promise-try) or by using a promise library such as bluebird or Q. – Ori Drori Sep 27 '16 at 20:55
  • 14
    @jfriend00 Obviously in this simple example the code will not behave differently. But what if you had code after the `reject` that does something expensive, like connect to databases or API endpoints? It would all be unnecessary and cost you money and resources, especially for example if you are connecting to something like an AWS database or API Gateway endpoint. In that case you would definitely use a return in order to avoid unnecessary code from being executed. – Jake Wilson Oct 20 '16 at 14:41
  • 5
    @JakeWilson - Of course, that's just normal code flow in Javascript and has nothing to do with promises at all. If you're done processing the function and don't want any more code to execute in the current code path, you insert a `return`. – jfriend00 Oct 20 '16 at 20:50
  • Agreeing with @JakeWilson. There might be more code between the `reject` and the `resolve`. Is it OK to do a `return reject();`? I assume `reject` simply returns `undefined` or something.. – Luke Jan 11 '17 at 00:09
  • @Luke - indeed - `reject()` returns `undefined`, and you can `return` it, if you need to break the flow, as jfriend00 described. – Ori Drori Jan 11 '17 at 06:38
  • 1
    @Luke it's good to return if there are other operations that could happen after. If you had a background async task (for example updating a database table) in your code after a `reject()`, you should make sure you `return reject(...)` or `return` after the `reject()`. `reject()` and `resolve()` only tell the Promise that the passed value is what it was waiting for. It does not stop code execution for that code block. – Jake Wilson Jan 12 '17 at 15:50
  • @Luke Keep the return on a separate line. Then you absolutely KNOW that you are returning undefined. Who *knows* what reject could be returning. – basickarl Dec 12 '17 at 16:18
  • @jfriend00 "resolve() after calling reject() won't do anything except use a few extra CPU cycles": if you a request or any other code after `reject()` it could be executed – BorisD Jul 30 '21 at 22:28
  • I HIGHLY recommend return after reject. Even more, if you use your reject as an "Exception traps" at the beginning of your function: if( text === null ) { reject(...exception/mesage...); return; }... rest of the funtion. I don't agree with with @jfriend00 – BorisD Jul 30 '21 at 22:33
  • @BorisD - What are you disagreeing with? Both of the two comments I authored here say that I would use the `return` in my code. – jfriend00 Jul 31 '21 at 02:29
48

A common idiom, which may or may not be your cup of tea, is to combine the return with the reject, to simultaneously reject the promise and exit from the function, so that the remainder of the function including the resolve is not executed. If you like this style, it can make your code a bit more compact.

function divide(numerator, denominator) {
  return new Promise((resolve, reject) => {
    if (denominator === 0) return reject("Cannot divide by 0");
                           ^^^^^^^^^^^^^^
    resolve(numerator / denominator);
  });
}

This works fine because the Promise constructor does nothing with any return value, and in any case resolve and reject return nothing.

The same idiom can be used with the callback style shown in another answer:

function divide(nom, denom, cb){
  if(denom === 0) return cb(Error("Cannot divide by zero"));
                  ^^^^^^^^^
  cb(null, nom / denom);
} 

Again, this works fine because the person calling divide does not expect it to return anything and does not do anything with the return value.

  • 20
    I do not like this. This gives the notion that you are returning something which you in fact aren't. You are invoking the reject function and then using return to end the function execution. Keep them on separate lines, what you are doing will only confuse people. Code readability is king. – basickarl Dec 12 '17 at 16:15
  • 11
    @KarlMorrison you're in fact returning "something", a rejected promise. I think that "notion" you're talking about is very personal. There's nothing wrong with returning a `reject` status – Frondor Mar 01 '18 at 01:35
  • 7
    @Frondor I don't think you understood what I wrote. Of course you and I understand this, nothing happens when returning a reject on the same line. But what about developers that are not so used to JavaScript coming into a project? This type of programming decreases readability for such people. JavaScript eco-system today is a mess enough and people spreading this type of practices will only make it worse. This is bad practice. – basickarl Mar 01 '18 at 14:09
  • 3
    @KarlMorrison Personal opinions != bad practice. It would probably help a new Javascript developer to understand what is going on with the return. – Toby Caulk Jan 30 '19 at 02:37
  • 3
    @TobyCaulk If people need to learn what return does then they shouldn't be playing about with Promises, they should be learning basic programming. – basickarl Jan 30 '19 at 10:54
  • @KarlMorrison It's the exact opposite. New Javascript developers most likely know what it means to"return" something, but they don't understand what just calling reject/resolve means yet. It's more explicit, I don't know why that would be such a big deal. – Toby Caulk Jan 31 '19 at 14:08
  • 1
    @TobyCaulk Exactly, most developers understand return but not reject. By returning a reject it makes junior developers think that reject returns a value from it's function call, which it doesn't. https://jsfiddle.net/tf9exkvn/ – basickarl Feb 03 '19 at 13:28
  • 1
    @Frondor Your comment is an excellent example of someone who is confused by this exact practice. You aren't returning something with return, you are returning something when invoking reject(), two totally different things. Here is some code for you: https://jsfiddle.net/tf9exkvn/ Also it's not personal, it's about getting junior developers up to speed when starting to work on large projects and making sure they understand promises fully. – basickarl Feb 03 '19 at 13:30
  • 1
    @basickarl Just to be 100% accurate. `return resolve(someVal)` is returning `undefined` which is being ignored by the `Promise` constructor which called that function. So, yes, it's confusing that the code looks like it's trying to return something but that returned value is `undefined` and is being ignored by the caller (the `Promise` constructor). I agree that the convenience of the return in one line is not worth the confusion... – Ruan Mendes Mar 25 '22 at 20:38
28

If you don't "return" after a resolve/reject, bad things (like a page redirect) can happen after you meant for it to stop. Source: I ran into this.

Benjamin H
  • 5,164
  • 6
  • 34
  • 42
  • 9
    +1 for the example. I had an issue where my program would make 100+ invalid database queries and I couldn't figure out why. Turns out I didn't "return" after a reject. It's a small mistake but I learned my lesson. – AdamInTheOculus Mar 23 '17 at 00:45
  • Another personal experience lol: rejecting an invalid id and not allowing it being "favorited" (in a "favorites list"), but not returning made it "both reject and get added to the list of favorites"! It didn't even reflect the truth at first, because there was an onError callback that manually updated the cache () and on the next render (e.g., when another item was requested to be added to the favorites list) it suddenly revealed the [sad] truth! – aderchox Feb 26 '23 at 09:25
14

Technically it is not needed here1 - because a Promise can be resolved or rejected, exclusively and only once. The first Promise outcome wins and every subsequent result is ignored. This is different from Node-style callbacks.

That being said it is good clean practice to ensure that exactly one is called, when practical, and indeed in this case since there is no further async/deferred processing. The decision to "return early" is no different than ending any function when its work is complete - vs. continuing unrelated or unnecessary processing.

Returning at the appropriate time (or otherwise using conditionals to avoid executing the "other" case) reduces the chance of accidentally running code in an invalid state or performing unwanted side-effects; and as such it makes code less prone to 'breaking unexpectedly'.


1 This technically answer also hinges on the fact that in this case the code after the "return", should it be omitted, will not result in a side-effect. JavaScript will happily divide by zero and return either +Infinity/-Infinity or NaN.

user2864740
  • 60,010
  • 15
  • 145
  • 220
9

The answer by Ori already explains that it is not necessary to return but it is good practice. Note that the promise constructor is throw safe so it will ignore thrown exceptions passed later in the path, essentially you have side effects you can't easily observe.

Note that returning early is also very common in callbacks:

function divide(nom, denom, cb){
     if(denom === 0){
         cb(Error("Cannot divide by zero");
         return; // unlike with promises, missing the return here is a mistake
     }
     cb(null, nom / denom); // this will divide by zero. Since it's a callback.
} 

So, while it is good practice in promises it is required with callbacks. Some notes about your code:

  • Your use case is hypothetical, don't actually use promises with synchronous actions.
  • The promise constructor ignores return values. Some libraries will warn if you return a non-undefined value to warn you against the mistake of returning there. Most aren't that clever.
  • The promise constructor is throw safe, it will convert exceptions to rejections but as others have pointed out - a promise resolves once.
Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
6

In many cases it is possible to validate parameters separately and immediately return a rejected promise with Promise.reject(reason).

function divide2(numerator, denominator) {
  if (denominator === 0) {
    return Promise.reject("Cannot divide by 0");
  }
  
  return new Promise((resolve, reject) => {
    resolve(numerator / denominator);
  });
}


divide2(4, 0).then((result) => console.log(result), (error) => console.log(error));
Dorad
  • 3,413
  • 2
  • 44
  • 71