gx0r/connect-session-knex

Bluebird warning caused by touch()

pmalouin opened this issue · 1 comments

I'm trying to upgrade an application from bluebird v2 to bluebird v3 but started seeing some bluebird warnings triggered by express routes. Here is a sample application that causes the warning:

'use strict';
var express = require('express');
var session = require('express-session');
var cookieParser = require('cookie-parser');
var KnexSessionStore = require('connect-session-knex')(session);
var knex = require('./DB').knex; //knex object is initialized in DB.js

var app = express();
app.set('port', process.env.PORT || 3000);

var server = require('http').createServer(app);
app.use(cookieParser());

var sessionStore = new KnexSessionStore({
    knex: knex,
    tablename: 'sessions', // optional. Defaults to 'sessions'
    createtable : false
});

var sessionConfig = {
    name : 'sessionid',
    resave : false,
    rolling : true,
    saveUninitialized : true,
    secret : 'some secret',
    store : sessionStore,
    cookie : {
        secure : 'auto',
        maxAge : 7200000
    }
};

app.use(session(sessionConfig));
app.get('/', function(req, res, next) {
    knex('users')
        .then(users => {
            // Deep inside res.json() a call to KnexSessionStore.touch() is made. Bluebird thinks we forgot to return
            // this Promise from this handler.
            res.json(users);
        })
        .catch(next);
});

// Start Express server.
server.listen(app.get('port'), function() {
    console.log('Express web server listening on port %d over %s in %s mode', app.get('port'), 'HTTP', app.get('env'));
});

The warning is:

(node:5088) Warning: a promise was created in a handler at /opt/apps/web/app.js:39:17 but was not returned from it, see http://goo.gl/rRqMUw
    at Promise.then (/opt/apps/node_modules/bluebird/js/release/promise.js:125:17)

The reason the warning appears is that when I call res.json(), deep down in the stack, a call to the session store's touch() method is made and the method creates a new Promise. Bluebird thinks that when a Promise is instantiated from inside a handler, it is likely that the developer forgot to return the Promise (to wait for its completion). In the present case, I cannot and do not wish to wait for .touch()'s completion.

Reading the Bluebird documentation about the warning, it seems that to prevent the warning from occuring, I should return null from the handler like this:

app.get('/', function(req, res, next) {
    knex('users')
        .then(users => {
            res.json(users);
            return null;
        })
        .catch(next);
});

Before I go over the many routes of my application to add return null statements, I was wondering if there is another way to solve this problem, if anybody has ever worked around this problem. Many thanks!

A not-so-great workaround would be to patch the store like this:

var sessionStore = new KnexSessionStore({
    knex: knex,
    tablename: 'sessions', // optional. Defaults to 'sessions'
    createtable : false
});

const originalTouch = sessionStore.touch;

sessionStore.touch = function(sid, sess, fn) {
    process.nextTick(function() {
        originalTouch.call(sessionStore, sid, sess, fn);
    });
};

Delaying the knex query to the next tick removes the Promise instantiation from the synchronous call to res.json().