A promise was created in a handler but was not returned from it
Znarkus opened this issue · 14 comments
Hello!
I'm getting plenty of this warning (using knex).
(node:7714) Warning: a promise was created in a handler at node_modules/tarn/lib/Pool.js:297:24 but was not returned from it, see http://goo.gl/rRqMUw
at new Promise (node_modules/bluebird/js/release/promise.js:79:10)
Is this a false warning or something that could potentially be fixed?
Could you write a reproduction for this? I don't understand how this could happen since Tarn doesn't even use bluebird. Are you possibly using a custom dialect for knex?
That's because override Bluebird as the global Promise :)
global.Promise = require('bluebird')Otherwise it's a basic knexjs setup according to the examples.
Well there you go 😄 You shouldn't overwrite globals like that.
So it's not an issue that a promise is created in a handler but is not returned from it?
Tarn's handlers like create can either return a promise or call the callback passed to them. Some of knex's dialects choose to use the callback style, while still using promises inside the handler. That's probably where the warning is coming from. The only way to fix this is to fix it in knex, but I'm pretty sure no one there will do anything to help you with this, since you are modifying global objects.
TLDR: It is an issue of promise created in a handler and not returned from it, but that's done intentionally. Native promise doesn't have that kind of warnings.
As long as it's not an issue I can live with the warning.
Overriding the global Promise is not uncommon petkaantonov/bluebird#1026
It being common doesn't make it any less extremely stupid 😉
Btw, this also creates a huge memory leak. I was able to debug it to the following. Basically looks like these unhandled warnings are piling up or something. Not exactly sure what. But memory grows steadily if global Bluebird is used as a Promise.
Took me several hours to come to this...
I know it's not Tarn's issue. But it'd be awesome if it could play along and play nice with global Bluebird.
"Error
at Promise.longStackTracesCaptureStackTrace [as _captureStackTrace] (...node_modules/bluebird/js/release/debuggability.js:411:19)
at Promise._resolveFromExecutor (...node_modules/bluebird/js/release/promise.js:480:10)
at new Promise (...node_modules/bluebird/js/release/promise.js:79:10)
at defer (...node_modules/tarn/lib/utils.js:9:19)
at new Resource (...node_modules/tarn/lib/Resource.js:10:21)
at Resource.resolve (...node_modules/tarn/lib/Resource.js:19:12)
at Pool.release (...node_modules/tarn/lib/Pool.js:129:29)
at Client_PG.releaseConnection (...node_modules/knex/lib/client.js:348:32)
at Client.<anonymous> (...node_modules/knex/lib/runner.js:236:28)
at FunctionDisposer.doDispose (...node_modules/bluebird/js/release/using.js:98:19)
at FunctionDisposer.Disposer.tryDispose (...node_modules/bluebird/js/release/using.js:78:20)
at iterator (...node_modules/bluebird/js/release/using.js:36:53)
at dispose (...node_modules/bluebird/js/release/using.js:48:9)
at ...node_modules/bluebird/js/release/using.js:194:20
at PassThroughHandlerContext.finallyHandler (...node_modules/bluebird/js/release/finally.js:56:23)
at PassThroughHandlerContext.tryCatcher (...node_modules/bluebird/js/release/util.js:16:23)
at Promise._settlePromiseFromHandler (...node_modules/bluebird/js/release/promise.js:512:31)
at Promise._settlePromise (...node_modules/bluebird/js/release/promise.js:569:18)
at Promise._settlePromise0 (...node_modules/bluebird/js/release/promise.js:614:10)
at Promise._settlePromises (...node_modules/bluebird/js/release/promise.js:694:18)
at _drainQueueStep (...node_modules/bluebird/js/release/async.js:138:12)
at _drainQueue (...node_modules/bluebird/js/release/async.js:131:9)"
It could maybe check that global promise is the native one and throw an error if not 🤔
Does your case leak memory with native promise or do you have test case to reproduce the leak? If there is no test case showing that leak is caused by tarn I would suspect that there is a bug in code using tarn.
I am getting the same error too, using knex together with Bluebird as the global promise library.
@koskimas, just wanted to find out: why is it not a good idea to override nodejs's global promise library with bluebird? Just want to get a better understanding of this.
same issue. Anybody can explain it?
It seems like I override global.Promise with bluebird Promise
@joshuaquek @mrdulin Everything that is using the native promise without importing a custom promise is expecting to get the native promise and nothing else. Overriding globals and breaking that expectation is the problem. This should be obvious.
As a hyperbole, what if I replace Array.prototype.splice with a custom implementation that doesn't support the third argument and then use libraries that use the third argument. Can you see the problem there?
This can be fixed by returning a null from those promise methods that now return undefined. I'd merge a PR that does this since people insist on doing stupid-ass things like overriding native globals.