Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluebird warning caused by touch() #39

Open
pmalouin opened this issue Aug 31, 2017 · 1 comment
Open

Bluebird warning caused by touch() #39

pmalouin opened this issue Aug 31, 2017 · 1 comment

Comments

@pmalouin
Copy link

pmalouin commented Aug 31, 2017

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!

@pmalouin
Copy link
Author

pmalouin commented Sep 1, 2017

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().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant