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.

Answer 1

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.