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

async/await Warning: a promise was created in a handler but was not returned from it #903

Closed
sahat opened this issue Dec 5, 2015 · 14 comments

Comments

@sahat
Copy link

sahat commented Dec 5, 2015

Here is my async function that does not return anything:

var fs = require('fs-extra');
var path = require('path');
var Promise = require('bluebird');

var copy = Promise.promisify(fs.copy);

async function generateExpress(params) {
  let build = path.join(__dirname, 'build', params.uuid);
  let express = path.join(__dirname, 'modules', 'express');

  await copy(express, build);
}

// await generateExpress(params) called later in the code.

It throws a bunch of warnings "a promise was created in a handler but was not returned from it". Previously, before switching to async/await, to get around this issue, I would add a return in front of every promise.

Is this a bug or working as intended by bluebird library?

screenshot 2015-12-04 21 28 06

@sahat sahat changed the title async/await Warning: async/await Warning: a promise was created in a handler but was not returned from it Dec 5, 2015
@phpnode
Copy link

phpnode commented Dec 5, 2015

@sahat which version of babel are you using?

@sahat
Copy link
Author

sahat commented Dec 5, 2015

@phpnode

Node 4.2.1
Mac OS X Yosemite

    "bluebird": "^3.0.6",

    "babel-core": "^6.3.13",
    "babel-loader": "^6.2.0",
    "babel-polyfill": "^6.3.14",
    "babel-preset-es2015": "^6.3.13",
    "babel-preset-react": "^6.3.13",
    "babel-preset-stage-0": "^6.3.13",

@sahat
Copy link
Author

sahat commented Dec 5, 2015

I was able to suppress the warning by adding this at the beginning of my file:

Promise.config({
  warnings: false
});

But the strange thing is, I thought warnings are OFF by default according to the docs?

Promise.config(Object {
    warnings: boolean=false,
    longStackTraces: boolean=false,
    cancellation: boolean=false
} options) -> undefined;

@phpnode
Copy link

phpnode commented Dec 5, 2015

@sahat babel6 has some issues with async functions at the moment, I would check whether you can replicate the issue using something like this without compiling:

var fs = require('fs-extra');
var path = require('path');
var Promise = require('bluebird');

var mkdirs = Promise.promisify(fs.mkdirs);
var copy = Promise.promisify(fs.copy);

var generateExpress =  Promise.coroutine(function *generateExpress(params) {
  var build = path.join(__dirname, 'build', params.uuid);
  var express = path.join(__dirname, 'modules', 'express');

  yield copy(express, build);

  yield mkdirs(path.join(build, 'public', 'images'));
  yield mkdirs(path.join(build, 'public', 'javascripts'));
  yield mkdirs(path.join(build, 'public', 'stylesheets'));
});

@zhm
Copy link

zhm commented Dec 10, 2015

FWIW, I'm seeing a very similar issue using co.

This code prints the same warning, but only twice. It does NOT print the warning when using Promise.coroutine though. Perhaps I'm doing something completely wrong. It only seems to happen when yielding more than once inside the generator. Here is a trivial example that reproduces this warning:

BLUEBIRD_DEBUG=1 node test.js
var Promise = require('bluebird');
var co = require('co');

function getPromise(val, err) {
  return new Promise(function (resolve, reject) {
    if (err) {
      reject(err);
    } else {
      resolve(val);
    }
  });
}

var generator = function *() {
  var a = yield getPromise(1);
  var b = yield getPromise(2);
  var c = yield getPromise(3);

  console.log(a, b, c);
};

// warnings
co(generator);

// ok
Promise.coroutine(generator)();

bluebird: 3.0.6
co: 4.6.0
node: 4.1.1

@nazrhyn
Copy link

nazrhyn commented Dec 10, 2015

I was going to create another issue for this, but I'm just going to add a comment here.

Some combination of jaredhanson/passport, jaredhanson/oauth2orize, bluebird (3.0.6) and our code is causing this warning to be raised any time the code paths through some of our functions. As far as I can see, none of those functions create promises that aren't returned.

The only common factor I can see is that the final function before the warning always has a Promise.try call in it. In one case, I'm returning the result and in the other, I'm just starting a promise chain that ultimately calls a callback function that's provided to me. I've tried reproducing this structure simply outside of the main codebase, but in all of my reproductions, the warning is not raised. I've gone through the entire call stack across the long stack trace and and tried putting return null at the end of every function that didn't explicitly return (even ones that don't even involve promises, just in case).

I'm not sure what else I can do other than disabling the warnings, as the logs aren't telling me enough to figure out why Bluebird thinks there's a promise created that wasn't returned. As I led with, I suspect it has something to do with the libraries I'm using in concert with the rest of the code; but, I'm not sure about that.

Is there any way to get more information than the warning currently shows? Also, there's a double space in the text of the warning that makes me think there's something that's supposed to be concatenated there, but isn't:

W | Warning: a promise was created in a  handler but was not returned from it

EDIT: If you think this is sufficiently unrelated, I'll create a separate issue for it. I was just concerned that I am basically giving you no information (because I don't have any, really); so, I didn't want to make a full issue, given that.

@petkaantonov
Copy link
Owner

The problem is that you cannot detect whether the immediate handler created the promise or some function the handler calls. A handler might call into 30 levels of functions where function 28 creates a promise, then if the handler doesn't have a return, it will be reported even though the handler code cannot actually do anything about it. Detecting that it was the immediate handler function that created the promise is impossible due to strict mode hiding callers from arguments.caller.

@nazrhyn
Copy link

nazrhyn commented Dec 11, 2015

Right. I can appreciate the limitations that Bluebird is bound by.

So, the question becomes: understanding that most code bases will interact with (for many intentions) opaque modules, with the advent of these warnings, what is the expected use pattern?

In my case, I have verified that none of my code is written improperly; so, moving forward, either we: (1) leave the warnings enabled, and they will be disabled when NODE_ENV is not development; or (2) disable them pre-emptively with Promise.config to reduce noise in our development logs. Perhaps the warnings should just be periodically turned on? What is your expectation? I can imagine the possibility that the logs could become quite noisy, depending on the modules with which one's code interacts.

@bergus
Copy link
Contributor

bergus commented Dec 11, 2015

Hm, I guess this could be fixed by the co library, as they are not returning the inner promises from onFulfilled/onRejected/next (here) while they theoretically could.
But they might be doing this deliberately, at least they also employ the Promise constructor antipattern - to avoid memory leaks with native V8 promises.

@EvHaus
Copy link

EvHaus commented Dec 15, 2015

babel6 has some issues with async functions at the moment, I would check whether you can replicate the issue using something like this without compiling:

@phpnode Has this been filed with the Babel team? I can't seem to find anything about it there.

They need to be made aware that their async generator code generates Promises incorrectly.

@phpnode
Copy link

phpnode commented Dec 16, 2015

@globexdesigns there are a lot of issues with async functions in babel6 but they're slowly getting fixed, here's just a few for example:

https://phabricator.babeljs.io/T2765
https://phabricator.babeljs.io/T3047
https://phabricator.babeljs.io/T2892
https://phabricator.babeljs.io/T2776

@petkaantonov
Copy link
Owner

I have experimented with a design that allows knowing if the handler was immediately creating a promise but not returning it. However, this has tremendous performance cost and it is trading off false positives for false negative. Additionally, false positives can always be dealth with by returning null while false negatives cannot be dealt with at all.

Example of false negative:

function returnsPromise() {
    return Promise.resolve();
}


somePromise.then(function() {
    // Forgot to return but no warning
    // because the promise is not immediately created here.
    returnsPromise();
});

@dashed
Copy link

dashed commented Dec 24, 2015

@zhm Out of curiosity, have you filled an issue with babel regarding your issue at #903 (comment) ? I'm also getting some weird interoperability issues between co (and generators) and bluebird.

@dantman
Copy link
Contributor

dantman commented Dec 24, 2015

I've discovered this issue happening allot when Bluebird using async code + loops are processed with regenerator resulting in a certain format of _asyncToGenerator being output.

I reported this on the babel bug tracker:
https://phabricator.babeljs.io/T6886

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

9 participants