Ending a Promise chain

Question

I am running into a Promise warning about an unterminated Promise chain ('a promise was created in a handler but was not returned from it'). I am new to Promises, and suspect I'm using non-event-driven thinking when I shouldn't be. But I'm not sure how to proceed. This is all within a nodejs project.

I am interacting with a ZWave server to turn lights on and off. That interaction takes the form of making http requests to a server which controls the ZWave network. I'm using Promises because of the asynchronous nature of interacting via http.

At one level of my program I have the following class method defined:

ZWave.prototype.onOff = function (nodeNumber, turnOn) {
var self = this;
var level = turnOn ? 255 : 0;

return new Promise(function (resolve, reject) {
    self.requestAsync(sprintf('/Run/devices[%d].instances[0].commandClasses[0x20].Set(%d)', nodeNumber, level))
    .then(function (value) {
        resolve(value == 'null');
    })
    .catch(function (error) {
        reject(error);
    });
});

};

The requestAsync class method is the one that actually handles interaction with the ZWave server. Conceptually, in onOff() I'm trying to turn a particular light, identified by this.nodeNumber, either on or off, and then return the result of that request.

onOff() is being called from a Switch class method, representing a particular light, as follows:

   this.onOff = function( turnOn ) {
    var state = turnOn ? 'ON' : 'OFF';

    var self = this;

    zWave.onOff( this.nodeNumber, turnOn )
    .then( function() {
        winston.info( sprintf( '%s: turned %s', self.displayName, state ) );
        return true;
    } )
    .catch( function( error ) {
        winston.info( sprintf( '%s: error while turning %s => %s', self.displayName, state, error ) );
        return false;
    } );
}

The 'return true' and 'return false' statements are my attempt to end the Promise chain. But it's not working, and I'm still getting the warning.

This strikes me as a specific example of a more general issue: how do you move from a Promise-based model to traditional blocking code?

Edit

Answering a few questions from comments...

I'm using bluebird.

zWave.onOff() returns a promise.

Switch.onOff() and zWave.onOff() are distinct functions in separate classes.

All the code is javascript.

Edit 2

I believe I have implemented jfriend00's suggestions, although I included a .catch() handler in the zWave.onOff() function, but I am still getting the same unhandled promise error:

ZWave.prototype.onOff = function (nodeNumber, turnOn) {
    var self = this;
    var level = turnOn ? 255 : 0;

    return self.requestAsync( sprintf( '/Run/devices[%d].instances[0].commandClasses[0x20].Set(%d)', nodeNumber, level ) )
        .then( function( value ) {
            resolve( value == 'null' );
        } )
        .catch( function( error ) {
            reject( error );
        } );
};

and

// function inside Switch class
this.onOff = function( turnOn ) {
    var state = turnOn ? 'ON' : 'OFF';
    var self = this;

    return zWave.onOff( this.nodeNumber, turnOn ).reflect()
        .then( function() {
            winston.info( sprintf( '%s: turned %s', self.displayName, state ) );
            return true;
        } )
       .catch( function( error ) {
            winston.info( sprintf( '%s: error while turning %s => %s', self.displayName, state, error ) );
            return false;
        } );
}

Here is the text of the warning:

Warning: a promise was created in a handler but was not returned from it at ZWave.requestAsync (/home/mark/XmasLights/zWaveRequest.js:19:12) at ZWave.onOff (/home/mark/XmasLights/zWaveRequest.js:93:17) at onOff (/home/mark/XmasLights/switch.js:42:22) at updateCron (/home/mark/XmasLights/switch.js:80:18) at dailyUpdate (/home/mark/XmasLights/app.js:196:21) at /home/mark/XmasLights/app.js:134:58 at processImmediate [as _immediateCallback] (timers.js:383:17)

Sorry about the run on formatting of the warning, I can't seem to get stackoverflow to separate the lines properly.

Are you using a Promise library like `q` or `bluebird`, or are these just plain node Promises? The possible answers differ greatly. — Patrick J. S., Dec 05 '15 at 17:07
@acupajoe: he won't be able to if he imported it via `var requestFn = bluebird.promisifyAll(require('some module'))` — Patrick J. S., Dec 05 '15 at 17:11
No `Promise` returned from `this.onOff` ? , e.g., `return zWave.onOff( this.nodeNumber, turnOn )` ? — guest271314, Dec 05 '15 at 17:15
Are `this.onOff` , `ZWave.prototype.onOff` same function ? Or two different functions with same name ? — guest271314, Dec 05 '15 at 17:23
Answering your question about "traditional blocking code": there is no blocking in JS (which is good as it would freeze the application), but you can use libraries like co: https://github.com/tj/co (which is pretty awesome) to write your code in that way. — m90, Dec 05 '15 at 17:39
You should just avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572) — Bergi, Dec 05 '15 at 18:01
Just make a habit of always returning something when using promises. No matter if you call a function just for side effects. Every single `.then` callback should end in a `return` statement. Slap yourself whenever you forget to add a `return` and go add a `return`. Begin your function by writing "return foo", then complete the function. Do it until forgetting to return gives you nightmares at night. This practice will preserve your sanity when debugging your programs. — kliron, Dec 05 '15 at 19:58
@kliron - there's no need to return a value from a `.then()` handler if you're done with the operation at that point and there is no return value. — jfriend00, Dec 05 '15 at 20:18
@jfriend00 of course not. But there is no need not to do it either. — kliron, Dec 06 '15 at 08:35
You said to ***make it a habit of always returning something***. That just doesn't make sense. You return a value if there's a useful value to return that somebody will use. Otherwise, you don't return anything. Promises don't have to return something. They can just be used as triggers for when async operations are done. — jfriend00, Dec 06 '15 at 09:08
@jfriend00 "make it a habit" is a **suggestion** to make a habit of something. Don't do it if you'd rather not. I never said he **has** to return something. In my mind it is good style if all functions return something but that's just my FP poisoned mind. — kliron, Dec 06 '15 at 14:19
And, I'm disagreeing that it's good style in Javascript to always return something when there is no return value that a caller would ever use. — jfriend00, Dec 06 '15 at 15:55
I've marked jfriend00's reply as the answer because I'm pretty sure the continuing error I was encountering after adopting his suggestion was due to making a similar mistake -- not returning something from a Promise -- in a different function. At least, when I added a return to that other function the error went away. — Mark Olbert, Dec 06 '15 at 18:33
@jfriend00 I don't mean you should return something just to return something. I meant that it is a good style to use functions that communicate with returned values instead of side effects, etc. Even more so when using promises, where a big proportion of bugs are caused because you have forgotten to return something. That Bluebird warning exists for a good reason. — kliron, Dec 07 '15 at 20:27
@MarkOlbert - I don't see what would cause that warning in your recent edited code. I think the issue is perhaps inside of `self.requestAsync()`. See this simple jsFiddle for what causes that error: http://jsfiddle.net/jfriend00/mct1xjs8/. Also, your newest edit to `ZWave.prototype.onOff()` simply won't work as you still have calls to `resolve()` and `reject()` in them but those are not defined any more. — jfriend00, Dec 07 '15 at 21:20
@MarkOlbert are you still using the Promise constructor as in the first function in your code example `ZWave.prototype.onOff`? If so, try adding a `return` in front of `self.requestAsync(..)`. Also, if `requestAsync()` already returns a Promise, there is no need for this antipatern as others have commended. The Promise constructor should be used to start the promise chain when working with APIs that don't use promises. — kliron, Dec 08 '15 at 06:48

jfriend00 · Accepted Answer · 2015-12-05T20:16:51.693

According to the Bluebird docs, this warning occurs when you create a promise inside a promise scope, but you don't return anything from that Promise scope. Bluebird detects that you create a promise inside a promise handler, but didn't return anything from that handler which is potentially an unlinked promise when it should be linked to other promises. You can read what they say about the issue here.

In the code you've disclosed, you don't show anything that is like the examples Bluebird shows in their doc, but I would guess that one issue is here:

ZWave.prototype.onOff = function (nodeNumber, turnOn) {
    var self = this;
    var level = turnOn ? 255 : 0;
    return new Promise(function (resolve, reject) {
        self.requestAsync(sprintf('/Run/devices[%d].instances[0].commandClasses[0x20].Set(%d)', nodeNumber, level)).then(function (value) {
            resolve(value == 'null');
        }).catch(function (error) {
            reject(error);
        });
    });
};

Where you would be better off avoiding the anti-pattern and doing this:

ZWave.prototype.onOff = function (nodeNumber, turnOn) {
    var level = turnOn ? 255 : 0;
    return this.requestAsync(sprintf('/Run/devices[%d].instances[0].commandClasses[0x20].Set(%d)', nodeNumber, level)).then(function (value) {
        return(value == 'null');
    });
};

There is no need to create a new promise here because you already have one that you can just return. You can read more about avoiding the type of anti-pattern you were using here.

And, another possible issue is here where you create a return promise value, but don't return it from the method this.onOff. You've explicitly done a return true; or return false; from your .then() handlers, but then you never do anything with that return value because there are no subsequent .then() handlers and the promise itself is not returned.

this.onOff = function( turnOn ) {
    var state = turnOn ? 'ON' : 'OFF';

    var self = this;

    zWave.onOff( this.nodeNumber, turnOn )
    .then( function() {
        winston.info( sprintf( '%s: turned %s', self.displayName, state ) );
        return true;
    } )
    .catch( function( error ) {
        winston.info( sprintf( '%s: error while turning %s => %s', self.displayName, state, error ) );
        return false;
    } );
}

I'd suggest changing that to return the promise like this:

this.onOff = function( turnOn ) {
    var state = turnOn ? 'ON' : 'OFF';

    var self = this;

    // ===== add return statement here
    return zWave.onOff( this.nodeNumber, turnOn )
    .then( function() {
        winston.info( sprintf( '%s: turned %s', self.displayName, state ) );
        return true;
    } )
    .catch( function( error ) {
        winston.info( sprintf( '%s: error while turning %s => %s', self.displayName, state, error ) );
        return false;
    } );
}

This, then provides the true or false returned promise value to the caller of .onOff(). Or, if you have no need to get those returned values, then you can remove the return true and return false statements so there is no promise value.

This strikes me as a specific example of a more general issue: how do you move from a Promise-based model to traditional blocking code?

You don't write traditional blocking code for async operations. Async operations are async and will never be traditional blocking code. Promises provide a lot more structure to managing and coordinating async operations, but you still have to write code for them in an async fashion. It's just easier to write that async code using promises.

The JS language is adding more capabilities such as generators and await which you can use to help with this. Here's a short article on that topic: ES7 async functions.

Very helpful, thanx! I'll post back when I've tried your suggestions. One question, though: the second suggestion doesn't seem to involve any changes to the code you said I should correct. What am I missing? — Mark Olbert, Dec 05 '15 at 19:53
@MarkOlbert - In the last code block in my answer, there is an addition `return` in front of `zWave.onOff()` to return the promise. — jfriend00, Dec 05 '15 at 20:17

Ending a Promise chain

1 Answers1