Bluebird: Warning: a promise was created in a handler but none were returned from it

[rayshan]

Hello, trying out the 3.0 branch. Have a question regarding this commit: https://github.com/petkaantonov/bluebird/commit/54b627d243556399c9b3f99555d2aaede34cb27c

Is it best practice to always return a promise created in a promise handler? What's the reasoning behind it?

[threehams]

I'm glad this was finally added. One of the most common mistakes when first using promises is something like this:

fs.readFileAsync('./stuff.txt').then(function(stuff) {
  requestAsync({url: 'http://localhost/users', method: 'POST'});
}).then(function() {
  // stuff you want to do only after the request is completed and successful
});

If you forget to return that request promise, the next handler executes immediately with an argument of undefined - which is completely valid for the Promises/A+ spec, but not what you want 99% of the time.

[petkaantonov]

What @threehams said, it's not about best practices but your code actually being broken. It is extremely unlikely that creating a promise inside handler but not returning it is what you want, because it creates a runaway promise that is not connected to any chain, therefore its errors cannot be handled and obviously the async operation cannot be awaited for by anything.

[rayshan]

Got it, makes a lot of sense. Thanks guys.

[aegyed91]

this warning will trigger where there is no search result?

User
      .findOne({ $or: $or })
      .exec()
      .then((user) => {

[petkaantonov]

@tsm91 huh? can you paste more complete example with the stack trace

[aegyed91]

ahh sorry, i thought a query with no results will trigger this warning message, but i tried it with different scenarios and this is not the case

at a specific case i am not returning the promise probably, but i could not figure it out where yet, i don't think so the problem is in passport.

here is my code and the stack trace:

// ## Facebook Authentication
  if (settings.facebook.enabled) {
    // web-based
    passport.use(new FacebookStrategy({
      callbackURL: settings.url + '/auth/facebook/callback',
      clientID: settings.facebook.appID,
      clientSecret: settings.facebook.appSecret
    }, providerAuthCallback)); // eslint-disable-line

    // token-based
    passport.use(new FacebookTokenStrategy({
      clientID: settings.facebook.appID,
      clientSecret: settings.facebook.appSecret
    }, providerAuthCallback)); // eslint-disable-line
  }

/**
   * A common oauth provider callback
   *
   * @param {String} accessToken  -
   * @param {String} refreshToken -
   * @param {Object} profile      - profile object with the requested information from the provider
   * @param {Function} done       - cb
   * @returns {*}
   */
  function providerAuthCallback(accessToken, refreshToken, profile, done) {
    if (
      profile.emails.length === 0
      || !_.isObject(profile.emails[0])
      || !validator.isEmail(profile.emails[0].value)
    ) {
      return done(new Error(util.format(LNG.NO_SOCIAL_EMAIL, profile.provider)));
    }

    let $or = [{ email: profile.emails[0].value }];

    // normalize the auth callbacks by simply pushing to the
    // $or query that will be executed with User.findOne below
    // this allows us to simply have one auth callback for
    // different providers like Facebook, Google, etc.
    let provider = {};
    provider[profile.provider + '_id'] = profile.id;
    // note that we unshift instead of push, since we want findOne
    // to return the user based off `profile.id` which takes
    // precedence over the user's email address in `profile.emails`
    $or.unshift(provider);

    User
      .findOne({ $or: $or })
      .exec()
      .then((user) => {
        if (user) {
          if (!user[profile.provider + '_id']) {
            user[profile.provider + '_id'] = profile.id;
          }

          if (accessToken) {
            user[profile.provider + '_access_token'] = accessToken;
          }

          if (refreshToken) {
            user[profile.provider + '_refresh_token'] = refreshToken;
          }

          return user.save(done);
        }

        let avatar;

        /* eslint-disable indent */
        switch (profile.provider) {
          case 'facebook':
            avatar = `https://graph.facebook.com/${profile.id}/picture`;
            break;
          case 'google':
            avatar = profile._json.image.url.split('?')[0];
            break;
          default:
            const hash = crypto.createHmac('sha1', settings.hashKey).update(profile.emails[0].value).digest('hex');
            avatar = `https://gravatar.com/avatar/${hash}?d=identicon`;
        }
        /* eslint-enable indent */

        let userInfo = {
          email: profile.emails[0].value,
          name: profile.displayName,
          gender: profile.gender || profile._json.gender,
          avatar: avatar,
          [profile.provider + '_id']: profile.id
        };

        if (accessToken) {
          userInfo[profile.provider + '_access_token'] = accessToken;
        }

        if (refreshToken) {
          userInfo[profile.provider + '_refresh_token'] = refreshToken;
        }

        // if the user signed in with another service
        // then create a random password for them
        User.register(userInfo, randomstring.token(), function(err, updatedUser) {
          if (err) {
            return done(err);
          }
          if (!updatedUser) {
            return done(new Error(LNG.SIGNUP_ERROR));
          }
          done(null, updatedUser);
          updatedUser.sendWelcomeEmail();
        });
      })
      .catch((err) => done(err));
  }

the stack trace:

Warning: a promise was created in a handler but none were returned from it
    at processImmediate [as _immediateCallback] (timers.js:368:17)
From previous event:
    at Strategy.providerAuthCallback [as _verify] (/Users/tsm/Sites/dd/etc/init/03-sessions.js:118:8)
    at /Users/tsm/Sites/dd/node_modules/passport-google-oauth/node_modules/passport-oauth/node_modules/passport-oauth2/lib/strategy.js:195:22
    at /Users/tsm/Sites/dd/node_modules/passport-google-oauth/lib/passport-google-oauth/oauth2.js:115:7
    at passBackControl (/Users/tsm/Sites/dd/node_modules/passport-google-oauth/node_modules/passport-oauth/node_modules/passport-oauth2/node_modules/oauth/lib/oauth2.js:125:9)
    at IncomingMessage.<anonymous> (/Users/tsm/Sites/dd/node_modules/passport-google-oauth/node_modules/passport-oauth/node_modules/passport-oauth2/node_modules/oauth/lib/oauth2.js:143:7)
    at emitNone (events.js:72:20)
    at IncomingMessage.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:903:12)
    at doNTCallback2 (node.js:439:9)
    at process._tickCallback (node.js:353:17)

[petkaantonov]

What is line 118?

Also mixing promises and callbacks like this is super confusing, you can just make normal promise chain and call .asCallback(done) at the end which will automatically map the promise chain properly to callback api.

[aegyed91]

line 118: .then((user) => {

yeah, this file is pretty much a mess, you mean like this?

return user
            .save()
            .asCallback(done);

[petkaantonov]

I mean doing:

User
  .findOne({ $or: $or })
  .exec()
  .then((user) => {
    if (...) {
        ....
        // Make sure it returns a promise
        return user.saveAsync();
    }

    ...
    // Make sure it returns a promise
    return User.registerAsync();
  })
  .asCallback(done)

Hold on, I will publish 3.0.4 which will makae the stack trace of forgotten return warning much more useful.

[petkaantonov]

Can you retry in 3.0.4 and see what the stack trace is now?

[aegyed91]

Warning: a promise was created in a handler but none were returned from it
    at Query.exec (/Users/tsm/Sites/dd/node_modules/mongoose/lib/query.js:2111:10)
    at Function.schema.statics.findByUsername (/Users/tsm/Sites/dd/node_modules/passport-local-mongoose/index.js:294:13)
    at Function.schema.statics.register (/Users/tsm/Sites/dd/node_modules/passport-local-mongoose/index.js:237:10)
    at /Users/tsm/Sites/dd/etc/init/03-sessions.js:171:14
    at processImmediate [as _immediateCallback] (timers.js:383:17)
From previous event:
    at Strategy.providerAuthCallback [as _verify] (/Users/tsm/Sites/dd/etc/init/03-sessions.js:118:8)
    at /Users/tsm/Sites/dd/node_modules/passport-oauth2/lib/strategy.js:195:22
    at /Users/tsm/Sites/dd/node_modules/passport-google-oauth/lib/passport-google-oauth/oauth2.js:115:7
    at passBackControl (/Users/tsm/Sites/dd/node_modules/oauth/lib/oauth2.js:125:9)
    at IncomingMessage.<anonymous> (/Users/tsm/Sites/dd/node_modules/oauth/lib/oauth2.js:143:7)
    at emitNone (events.js:72:20)
    at IncomingMessage.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:905:12)
    at doNTCallback2 (node.js:450:9)
    at process._tickCallback (node.js:364:17)

[petkaantonov]

Yea that will go away when you use the promise returned by User.register

[aegyed91]

The problem is there is no registerAsync function, this register method on user model is not a built in mongoose thing, this is from https://github.com/saintedlama/passport-local-mongoose/blob/master/index.js#L226-L258

i also promisified it in the user model

import passportLocalMongoose from 'passport-local-mongoose';
Promise.promisifyAll(passportLocalMongoose);

I also use mongoose.Promise = require('bluebird'); so i think mongoose model.save is okay, i dont need saveAsync.

but the main problem here is this register method

[petkaantonov]

Oh I see now. Mongoose is creating promises even when the callback api is used. And because you assigned mongoose.Promise = require("bluebird") they will create bluebird promises.

Instead of doing mongoose.Promise = require("bluebird") assignment, you should do Promise.promisifyAll(require("mongoose")) and use the Async suffixed methods.

[petkaantonov]

Alternatively you can also promisify the "passport-local-mongoose" statics and use return registerAsync

[aegyed91]

yes, so basically if you can do user.save(cb[err, user]) or return user.save().then(user).catch(err)

but is there any difference between promisify-ing the mongoose lib or use this feature as of mongoose 4.1.0 to be able to specify which promise library to use? https://github.com/Automattic/mongoose/issues/2688

[benjamingr]

@tsm91 meh: https://github.com/Automattic/mongoose/issues/2688#issuecomment-104422889

[petkaantonov]

You will not see any warnings that are out of your control if you promisify mongoose. If mongoose internally makes mistakes you will see warnings that you can't do anything about if you assign mongoose.Promise to bluebird.

[petkaantonov]

what do you mean by help? It gets rid of any warning such as this one which is out of his control

[aegyed91]

so basically i should drop this feature and do Promise.promisifyAll(require("mongoose")) .

it shouldnt be hard to append an Async to all affected methods, i hope i wont break anything

[benjamingr]

@petkaantonov the mongoose people are usually pretty responsive, maybe we should talk to them.

Stuff like https://github.com/Automattic/mongoose/blob/master/lib/query.js#L2101-L2127 is mostly wasteful. It's an interesting case anyway.

[petkaantonov]

Asking other libraries to change when they haven't really done anything wrong is not scalable and is pretty silly

[benjamingr]

Of course, but in this particular case - Mongoose only recently implemented their own promise support and they'd be interested in hearing about it. That's all I'm saying.

[aegyed91]

Meh, for now i will use mongoose.Promise = require('bluebird') I have more than 50 queries.

Anyways i just finished fixing my third bug where i don't return a promise in the chain thanks to this warning feature. It is really helpful.

[benjamingr]

It might be useful to be able to define a filter.

[petkaantonov]

@benjamingr you already can, e.g. return null. It is documented even http://bluebirdjs.com/docs/warning-explanations.html#warning-a-promise-was-created-in-a-handler-but-none-were-returned-from-it

User.register(...);
return null;

[benjamingr]

@petkaantonov I mean filtering code areas where you don't want to get warnings from. This is typically called "Just my code".

[rishabhmhjn]

Does it really has to be a null?

This one is working

return new Promise(resolve => {
  // do my calculations and get result
  resolve(result);
  return null;
});

Is it not OK if I do something like this ↓

This one isn't

return new Promise(resolve => {
  // do my calculations and get result
  return resolve(result);
});

[benjamingr]

@rishabhmhjn the return value of the promise constructor is ignored. Bluebird is right about warning you here - your return has no effect.

[c0debreaker]

We're getting the same error too. Adding the return null didn't help at all. We are using ampersand's fetch which don't return a promise.

    // get meta data about the catalog
    metaData = new CatalogMetaData( {id: this.id} );
    this.promises.metaData = new Promise(function(resolve, reject){
      metaData.fetch({
        success: function(){
          resolve(metaData);
          return null;
        },
        error: function(err){
          reject(err);
          return null;
        }
      });
    });

[wanming]

Same error when using co:

'use strict';

const db = require('../model/');

const File = {
  *index() {
    const count = yield db.File.count();
    yield this.render('index', { data: 'Hello, world!', count });
  },
};

module.exports = File;

Nothing changed after I add some data to the table files. Any updates?

[petkaantonov]

Is co creating bluebird promises? In any case you can just swap co for Promise.coroutine if you're using bluebird anyway.

[magicdawn]

I Got the same warning, and it warns a lot. I step into co, found out that

co(function*(){
  yield a_bluebird_promise;
  ...
})

1. we yield a_bluebird_promise
2. next(a_bluebird_promise) -> a_bluebird_promise.then(onFulfilled, onRejected) https://github.com/tj/co/blob/4.6.0/index.js#L100
3. next() in the onFulfilled() fulfill handler create a Promise and did not return https://github.com/tj/co/blob/4.6.0/index.js#L85

So, it's co's problem~ Or we should modify the warning strategy? Not when a handle create a promise but not return it, but when a case like this is continued with a then call

e.g

not warn this

awesomePromise
.then(function(val){
  doSomeThing(val); // return a promise, but not returned
})

warn a next then call

awesomePromise
.then(function(val){
  doSomeThing(val); // return a promise, but not returned
})
.then(function(){
  // blabla
})

cause previous then call create a promise but not returned, warn on this then call

[magicdawn]

BTW, using [email protected] [email protected]

[petkaantonov]

why use co when bluebird already comes with Promise.coroutine

[magicdawn]

maybe co.wrap is shorter than Promise.coroutine :joy:

[petkaantonov]

var co = require("bluebird").coroutine :P

[petkaantonov]

Your suggestion is very interesting, I'll consider it

[benjamingr]

This _is_ a problem since the chain is detached so I'm +1 on keeping the current strategy - but I'd love for a stronger way to opt-out of warnings (for example, we could check in the stack trace if the error originated in node_modules and if it did not present it).

[magicdawn]

Fixed in co, see tj/co#259, not published yet

[devotox]

I get this error when i do something like

promise.resolve(result).then(function(result){
            var _code = ( result && result.code ) || response_status.internal_success;
            return response.status(_code).json( ( result && result.response ) || result || []);
        }).catch(function(result){
            var _code = ( result && result.code ) || response_status.internal_error;
            return !response.headersSent ? response.status(_code).send(get_error_message(_code)) : get_error_message(_code);
        });

and even a return before the promise.resolve gives me nothing
Stack trace

api_1             | [ Nodemon ][ Error ] Warning: a promise was created in a handler but was not returned from it
api_1             |     at Object.__default__ (/var/app/current/api/common/utils/api_response_handler.js:181:18)
api_1             |     at Object.exports.switch_case (/var/app/current/api/common/utils/utils.js:77:51)
api_1             |     at Seneca.<anonymous> (/var/app/current/api/common/utils/api_response_handler.js:215:17)
api_1             |     at act_done (/var/app/current/api/node_modules/seneca/seneca.js:1220:21)
api_1             |     at /var/app/current/api/node_modules/gate-executor/gate-executor.js:155:20
api_1             |     at /var/app/current/api/common/utils/api_response_handler.js:262:12
api_1             |     at processImmediate [as _immediateCallback] (timers.js:383:17)
api_1             | From previous event:
api_1             |     at Object.__default__ (/var/app/current/api/common/utils/api_response_handler.js:257:27)
api_1             |     at Object.exports.switch_case (/var/app/current/api/common/utils/utils.js:77:51)
api_1             |     at null.callback (/var/app/current/api/common/utils/api_response_handler.js:322:18)
api_1             |     at Query.handleReadyForQuery (/var/app/current/api/node_modules/pg/lib/query.js:89:10)
api_1             |     at null.<anonymous> (/var/app/current/api/node_modules/pg/lib/client.js:163:19)
api_1             |     at emitOne (events.js:95:20)
api_1             |     at emit (events.js:182:7)
api_1             |     at Socket.<anonymous> (/var/app/current/api/node_modules/pg/lib/connection.js:109:12)
api_1             |     at emitOne (events.js:90:13)
api_1             |     at Socket.emit (events.js:182:7)
api_1             |     at readableAddChunk (_stream_readable.js:153:18)
api_1             |     at Socket.Readable.push (_stream_readable.js:111:10)
api_1             |     at TCP.onread (net.js:529:20)

[spion]

FWIW I agree that the warning is a bit too opinionated and it seems like the suggestion proposed with @magicdawn should reduce false (or rather "opinionated") positives to zero

[mathieug]

I get the warning with:

userSchema.statics.createUser = function() {
  const User = this || mongoose.model('User');

  return User.create({
    // ...
  });
};

Update: It came from co, I've replaced it by bluebird.coroutine(). Thanks @petkaantonov.

[benjamingr]

Quoting Gorgi from FB:

if you aren't attaching another handler then you cannot possibly care to wait for the operation

That makes sense, I changed my mind and I'm 100% with the change of the warning strategy.

Cheers.

[petkaantonov]

@magicdawn I added your suggestion in 3.3.4

[magicdawn]

Nice :smile:

[nfantone]

I realize this a closed issue, but since it has had activity in the last few days, I'm going to comment on this.

I'm experiencing this same warning. Happens in the following snippet, inside a class function:

    authenticate() {
      return (args, done) => {
        return this.strategy.authenticate(args)
          .then((profile) => provider.jwt(this.seneca, profile))
          .then((token) => done(null, { // <-- This line triggers the warning
            ok: true,
            token: token
          }))
          .catch((err) => done(null, {
            ok: false,
            why: err
          }));
      };
    }

And I don't see who's the culprit here. done is just a callback. And even if it weren't, I _am_ returning it. What's going on here, @petkaantonov?

[petkaantonov]

calling done is calling some other function that return a promise

[petkaantonov]

Basically something like this is happening:

function somethingElse() {
    return new Promise(...)
}

function something() {
    somethingElse()
}

.then(function() {
    something();
})

You can just use asCallback:

authenticate() {
      return (args, done) => {
        return this.strategy.authenticate(args)
          .then((profile) => provider.jwt(this.seneca, profile))
          .then((token) => {
            return {ok: true, token: token};
          })
          .catch((e) => {
            return {ok: false, why: e};
          })
         .asCallback(done);
      };
    }

[nfantone]

Thanks for your reply, @petkaantonov.

A few pointers:

In your first snippet, I think this

.then(function() {
    something();
})

Should really be return something();, to match my scenario.

But, that aside, if this is what's actually happening, I tend to think that the warning is not properly in place since done is a callback from some library that I do not control. Nor should I really care about it. Refactoring my own code base to get rid of a warning of yet another library sounds fishy, even if it is truthful.

Any thoughts?

[benjamingr]

To clarify: Your code is not at fault - the library you're using is.

I think we should filter warnings originating in node_modules (that is, the offending code itself is in node_modules)

[petkaantonov]

@nfantone what's the stack trace like?

[petkaantonov]

@benjamingr That's not distinguishable from:

.then(function() {
    callModuleThatReturnsPromise();
});

[petkaantonov]

@benjamingr The promise is also most like created in his code, where calling the done callback calls back again into his code.

[nfantone]

To clarify: Your code is not at fault - the library you're using is.

That seems to be the case, yes.

I think we should filter warnings originating in node_modules

Here's an idea: turn all of these "better practices" and suggestions into a linter of sorts. A separate tool that one could run to inspect own code and receive feedback. Then, you could throw any warnings or error you want, without polluting user's logs. And it'd be up to the user to do something about it or not.

Of course, this is the harder approach. But I don't think that swallowing potential, bug-causing warnings is the beat option, either.

[benjamingr]

None of the errors show in release mode and all of them are trivially opt-out with Promise.config. For every person complaining about them I talk to a hundred it found bugs for.

The problem is that this sort of thing is not statically analyzable- you have to run the code to find the problems.

[nfantone]

For every person complaining about them I talk to a hundred it found bugs for.

Agreed. They _are_ very useful. I'm not suggesting to get rid of them, at all.

you have to run the code to find the problems.

Then, run it. Of all the possible issues with my idea, that doesn't need to be one.

[benjamingr]

I don't understand what you're suggesting but feel free to create a PoC illustrating what you're asking for.

[jscheid]

None of the errors show in release mode and all of them are trivially opt-out with Promise.config. For every person complaining about them I talk to a hundred it found bugs for.

I agree fully with this approach, but what would be great is if false positives could be suppressed individually so that the signal doesn't drown in the noise. Is that already possible?

[tremby]

I'd also like a way to suppress false positives.

[tremby]

Ah, I just found the answer at the bottom of this page: return null or similar (rather than undefined) to suppress the warning.

[jscheid]

Thanks @tremby, I had missed that. I've managed to suppress the warnings on a case by case basis by making sure no undefined value is returned.

(I think there's room for improvement in the debug output. The warning alone didn't point me at the offending promise, I had to hack checkForgottenReturns to print the rejection and fulfillment handlers. Perhaps I didn't look close enough?)

[Toflar]

Sorry to chime in here. I love the warning as such because it really helped me spotting one or two issues beforehand! One thing I don't get, though (and I may be wrong because I'm fairly new to promises), is that it seems like I also get the warning when I use Promise.resolve() with a value instead of a thenable or another promise.

I have a translation routine which requests the server for translations and as this is asynchronous, I used your fantastic library (especially because of the cancellation feature). Bluebird now lets me know that I have the following warning: Warning: a promise was created in a handler but was not returned from it at translate ..... My translate basically looks like this (simplified):

var cache = {};

function translate(key) {
    // Check the cache and resolve immediately if available
    if (undefined !== cache[key]) {
        return Promise.resolve(cache[key]);
    }

    return request.createRequest('/translation/' + key) // This is a Promise too
        .then(function (response) {
            // Cache
            cache[key] = response.value;
            return response.value;
        });
}

The line where Bluebird emits the warning is the return Promise.resolve(cache[key]); in case there is something in the cache.
It's very well possible that I did something wrong on my side (actually I do think it is :-D) but maybe one of you guys can spot an error or check whether Bluebird does not consider the case when a promise is resolved with a value directly when printing that warning? Thanks anyway for the fantastic work!

[buraktamturk]

@Toflar

off topic, but I were in there when you posted it. You can also think about caching the promise, this way if someone else call translate('hello world') too while the other one is not resolved yet there would be one request & promise, they would wait same promise without two request to the server.

var cache = {};

function translate(key) {
    // Check the cache and resolve immediately if available
    if (undefined !== cache[key]) {
        return cache[key];
    }

    return cache[key] = request.createRequest('/translation/' + key) // This is a Promise too
        .then(function (response) {
            return response.value;
        });
}

I don't know whatever this is a good practice or bad one, but this is what I would do.

[Toflar]

Hi @buraktamturk, thanks for your input which is very much appreciated! In my case, this is not possible as - as I said this is simplified - I need to replace placeholders and actually load a whole translation domain, not just a single key etc.
However, I think there's really something strange going on here. Even if I use a simple

return Promise.resolve(jQuery.ajax(url, props))

the warning pops up again.

[benjamingr]

Looks like a bug

[koutsenko]

Hello, I'm using Sequelize, PassportJS, Express 4. Node version is 0.10.35.

I'm tired of warnings:
"Warning: a promise was created in a handler but was not returned from it."

It appears in the new user registration route while saving user access token to database and redirecting to another page. Code provided below is the last middleware function in the chain. I tried to add return null at the last function line outside of user.save call, but it's doesn't help!

function(request, response) {
  var back = url.parse(request.header('Referer')).pathname;    
  if (!request.body.local_memory) {
    return response.redirect(back); 
  }
  var token = uid(64);
  request.user.local_token = token;
  request.user.save().then(function() {
    response.cookie('local_memory', token, { 
      path: '/', 
      httpOnly: true, 
      maxAge: 604800000  // 7 days 
    });
    return response.redirect(back === '/registration' ? '/' : back);
  }, function(err) {
    return response.redirect(back === '/registration' ? '/' : back);
  });   
}

Seems that Express 4 and 5 are still doesn't support promises.
KoaJS requires the newer NodeJS version, so I can't switch to that until RedShift doesn't provide stable cartridge for that.

Please, what can I do? Where is the problem?
Suppress Bluebird warnings isn't the solution - they helps while debugging...
Thnx a lot.

[philgruneich]

Thank you for this discussion, it helped me a lot. I was trying to use promises from an Express 4 middleware, like:

Promise.resolve(item).then((item) => {
  if (!item) {
    throw new Error('Trigger the catch');
  }
  if (item.length) {
    // Do something
    return next();
  }

  return Promise.resolve('somethingelse')
    .then(() => { return next(); })
})
.catch(err => res.redirect('back'));

I'm not entirely sure why this triggered the notorious warning because next is indeed a function, but next() returns undefined, perhaps you can enlighten me.

I figured 2 ways to get my middleware to work after this thread and any input is welcome:

asCallback(next)

Promise.resolve(item).then((item) => {
 // Error
  if (item.length) {
    // Do something
    return;
  }
  return Promise.resolve('somethingelse')
    .then(() => { return; })
})
// .catch
.asCallback(next);

next(); return null;

Promise.resolve(item).then((item) => {
 // Error
  if (item.length) {
    // Do something
    next(); return null;
  }
  return Promise.resolve('somethingelse')
    .then(() => { next(); return null; })
})
// .catch

If these are the ideal approaches for these scenarios, I'm sure you can confirm it to me and this may serve as an example to others.

[gsccheng]

Okay, here is my situation. I'm using the latest versions of Express and Bluebird at the moment. I was getting this warning at the return line from code like this:

import Promise from 'bluebird';

export default (req) => {
    return Promise.resolve(req.user || null);
};

So, previous recommendations of returning null or using .asCallback wasn't helpful. I found out this warning only arose when I executed this function AND had an active session in Passport– which I don't know why yet. Perhaps Passport created some Bluebird promise that hasn't been returned yet and got confused when this function created another one? Functionally my app is fine.

For me I found out there are two ways to eliminate this warning. One is to use the native Javascript Promise instead. The second is to use this code to suppress it:

import Promise from 'bluebird';

Promise && Promise.config({
    // Enables all warnings except forgotten return statements.
    warnings: {
      wForgottenReturn: false
    }
  });

Hope this helps someone or someone can better explain why this is happening.

[bergus]

@gsccheng You need to show us where that default-exported function is called, not where it is defined. Creating the promise is not the problem, it's the caller of the function who doesn't chain onto the created promise.

[andreialecu]

Seeing the same warning pop up from an innocent return Promise.resolve()

Looks like a bug.

[petkaantonov]

is the caller returning the promise? is the caller's caller returning the promise? etc. Do you even have control of all of the caller's in the entire call chain?

[petkaantonov]

The warning has been improved in 3.4.3

[benjamingr]

@petkaantonov I think the issue was with https://github.com/petkaantonov/bluebird/issues/508#issuecomment-214339207

[benjamingr]

I can't reproduce this at all http://jsfiddle.net/b5vpcsra/

[maa105]

submitted a pull request regarding this issue:
https://github.com/Automattic/mongoose/pull/5151