4

In a Node.js environment if I do this:

var doNoResolve = true;

function a() {
    return new Promise(resolve => {
        if (doNotResolve) {
            return
        }
        resolve(10);
    });
}

a().then(() => {
    // I don't want this getting fired
});

On an incoming request, is this a memory leak? If I was using a plain old callback everything would turn out just fine if I didn't execute whatever callback was supplied, but this feels like it might not be... the very name promise implies this is somewhat wrong.

If I had to I could return a "fake promise" (return { then: () => {} }) inside function a() rather than a "real promise" if doNotResolve was true, but that feels a bit gross.

The particular use-case is that of an isomorphic React.js application where I don't want HTTP requests actually getting made (but I do want my stores to update to a state that causes, say, a loading icon to appear).

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Joshua
  • 6,320
  • 6
  • 45
  • 62
  • See [this similar question](http://stackoverflow.com/questions/31521194/does-javascript-promise-create-memory-leaks-when-not-rejected-or-resolved). – jib Jul 25 '15 at 18:10

1 Answers1

4

Why would you do that instead of rejecting?

The benefit of promises is that they allow both resolving and rejecting, which:

  1. Doesn't fire the then handler (unless you provide two callbacks, which is considered bad practice)
  2. Does fire the catch handler, which explicitly handles errors
  3. Still fire the finally handler

You can simply do:

function a() {
    return new Promise((resolve, reject) => {
        if (doNotResolve) {
            reject(new Error('Oh noes!'));
        }
        resolve(10);
    });
}

Any good Promise implementation will give you a stacktrace from the location you called reject to help you debug async code, as well as calling any catch/finally handlers:

a().then(val => {
  console.log('Got data:', val);
}).catch(err => {
  console.error(err);
}).finally(() => {
  console.log('Done!');
});

Never rejecting or resolving a promise will, depending on your implementation, leave it on the stack of pending promises and very likely throw or log an error when your page unloads or the node promise exits. I know Bluebird will complain if you've left any promises pending, since it typically indicates a bug within the async part of your code.

ssube
  • 47,010
  • 7
  • 103
  • 140
  • "unless you provide two callbacks, which is considered bad practice" Do you have any articles that supports that claim ? I'm intrigued. – elad.chen Jul 24 '15 at 15:26
  • @elad.chen I remember seeing something about that in the Bluebird API docs, but can't find it at the moment. I'll cite something when I figure out where I read it, but `getPromise().then(x).catch(y)` is much clearer than `then(x).then(null, y)` and slightly better than `then(x, y)`. – ssube Jul 24 '15 at 15:34
  • Because I don't want _anything_ to happen at all (at which point I guess maybe I shouldn't be using a "promise"?). I guess I could reject it and then catch it and do nothing in the catch, but doesn't seem much better – Joshua Jul 24 '15 at 15:36
  • @Joshua Rejecting without a `catch` handler would be correct, I believe. You still need to tell the promise that it's finished, one way or another, but nothing is asking you to handle that rejection. Using a promise without a `then` or `catch` is perfectly reasonable. – ssube Jul 24 '15 at 15:37
  • If I reject it and don't catch it an error will propagate and the process will crash... – Joshua Jul 24 '15 at 15:38
  • I think that would cause it to crash anyway...? – Joshua Jul 24 '15 at 15:45
  • No @ssube you're totally right, my assumption that uncaught promises in a node environment cause the process to die was incorrect. Thanks a lot – Joshua Jul 24 '15 at 16:01