Bluebird: Bluebird promises are catching exceptions for me

Created on 27 Aug 2015  路  13Comments  路  Source: petkaantonov/bluebird

I'm using modules (knex, asyncawait) that use Bluebird Promises, but not using Bluebird directly myself.

I'm running code that I've wrapped in a library that uses Node's domains to provide context/catch errors within that context. But what is happening is that the Bluebird promise involved internally within knex (for example) is catching any exceptions within knex with the message: "Possibly unhandled" before it can bubble up to the domain layer's exception handler.

Is there something that I'm doing wrong/something I can do to prevent this behaviour? As it stands, the exception occurs, Bluebird tweets about it being unhandled and I never get a chance to actually deal with the exception, as it doesn't seem to be rethrown/bubbled after Bluebird sees it.

Most helpful comment

From Bluebird's main page: "It is an anti-pattern in every other language to use catch-all handlers because they swallow exceptions that you might not know about."

But this is exactly what Bluebird is doing! It's acting as a catch-all for any exceptions that occur in code that it wraps! I don't want you to catch my exceptions for me, I want to deal with them myself! And even Bluebird's .catch handler is wrapped internally so that if I rethrow from the .catch handler, Bluebird treats it as "unhandled" all the same.

"the idea is to avoid needlessly crashing the process" So instead I never know that the exception even occurred in the first place, and the process keeps on chugging along, swallowing exceptions. Every single piece of advice says not do do what Bluebird is doing; catching exceptions and then handling them by not handling them!

All 13 comments

When I remove the asyncawait layer, my purposeful error in a knex operation causes an exception that results in "Unhandled rejection" with no rethrow/bubbling.

If you think that this issue is specific to knex's & asyncawait's implementation of Bluebird, then please let me know and just close this issue. :)

Promises catch exceptions from the outer layers of their wrapper libraries. The contract of promise-based fucntions is that they should never throw and instead return rejected promises.

Curious, is there any particular reason why you're using domains? Promises have their own (safer) error handling model which should generally make domains unnecessary...

We're using mainly using domains to wrap up each connection into a nice tidy context that handles exceptions that that connection generates/to provide connection specific data to non-connection specific functions. It's mainly something that the company I work for is looking into, and not something that we're trying to use in production.

The behaviour that we're going for is that if an exception occurs during a connection, the domain handling that connection will handle the exception and prepare the process for a restart after other connections are finished/drained. The goal is that the server gracefully restarts on an unhandled exception and we are notified about it through logs/alerts.

The problem is that if modules that use bluebird promises are not explicitly given a .catch handler, they seem to completely stop the propagation of an exception, which is pretty crazy from my point of view. I would never know that it had happened.

I believe we can override this functionality by providing our own onPossiblyUnhandledRejection etc and just rethrowing the exception there? Although we cannot do this as we do not control the modules that use bluebird.

It seems that this is an implementation specific issue, so I don't mind if this issue is closed as from what I can see, bluebird seems to be functioning as designed. :)

Aaaand I just found that I can override it globally using process.on! I'll close the issue now, but feel free to comment on our methods if you think that we could do things a better way. I know these promises are built to keep things all tied up in a neat little package, but sometimes developers forget to drop a .catch handler in and then the problem is only found when that particular code path experiences an exception...

...

Aaaand, I've found I'm still having a problem with "Possibly unhandled", which isn't caught by the handler for "unhandledRejection". What is the bottom line for dealing with bluebird hiding exceptions that occur when there is no .catch handler? We're meant to rely on someone seeing the "Possibly unhandled" message on the console during testing?

Yes, you can can change the global rejection handler to add logging. Restarting the process should not be strictly necessary though, as limitations caused by the way domains handle errors do not apply to promises (much easier to reason about cleanup and resource management)

On that topic, not sure if they're applicable to your problem, but you might want to check out bluebird's resource cleanup utilities

So Bluebird's reaction to a developer failing to include a .catch (or .done) handler is to log a stack trace out to the console and then swallow the exception completely? I'm not sure if I'm understanding it correctly.

I've just been testing just by throwing inside of my own Bluebird promise, as well as by rejecting with an Error (to the same end).

Sorry for being a pain in the butt. :/ I think part of the problem is that adding a global handler for 'unhandledRejection' allows me to propagate the exception from knex, but the asyncawait library uses Bluebird version 2.3.11 which does not seem to respond to this global handler (and so just swallows the exception with a 'possibly unhandled' message).

@Roam-Cooper not sure what you mean by swallow - if you mean not crashing the process, then yes.

Which asyncawait library is that? https://github.com/yortus/asyncawait seems to be using fibers, so it doesn't look like thats the one

Edit: apparently thats the one. Might not be a bad idea to ping the author to update the dependency - it _should_ be fully backward-compatible

I mean that the exception never propagates to an exception handling layer that wraps the promise itself, a global error handler would be an example, my domain that wraps the promise is another.

Ah yes, you're right, it is that library and it is using fibers, but also calls for "bluebird": "~2.3.11" I believe? I could be mistaken about whether or not it is wrapping itself with Bluebird promises but having searched through that version of Bluebird I can see that "CapturedTrace$PossiblyUnhandledRejection" is the function leaving the "Possibly unhandled" message. Whereas newer version of Bluebird seem to leave an "Unhandled rejection" message from the function "CapturedTrace.fireRejectionEvent".

It seems as though failing to crash, but also failing to except is a feature of Bluebird? If so, I'll find a work-around that works for us and post it here in case future peeps decide that that's what they want.

Thanks for your help. :) Bluebird has been so performant for us otherwise!

@Roam-Cooper the idea is to avoid needlessly crashing the process as this can often lead to denial of service and instead use tools such as the ones mentioned above to manage resources. (using is analogous to C# using and Python with)

2.7 and up has the global handler (useful to introduce a different logging scheme or even to crash the process if the above doesn't sound convincing)

From Bluebird's main page: "It is an anti-pattern in every other language to use catch-all handlers because they swallow exceptions that you might not know about."

But this is exactly what Bluebird is doing! It's acting as a catch-all for any exceptions that occur in code that it wraps! I don't want you to catch my exceptions for me, I want to deal with them myself! And even Bluebird's .catch handler is wrapped internally so that if I rethrow from the .catch handler, Bluebird treats it as "unhandled" all the same.

"the idea is to avoid needlessly crashing the process" So instead I never know that the exception even occurred in the first place, and the process keeps on chugging along, swallowing exceptions. Every single piece of advice says not do do what Bluebird is doing; catching exceptions and then handling them by not handling them!

This is why you have the process global hook - the problem is that crashing diverges from ES2015 promises behavior.

This is an opinionated choice to make so the library leaves it up to you.

For what it's worth domains are deprecated in Node and will be removed in the future, I agree with Gorgi that using them is unnecessary here.

@Roam-Cooper I'm not sure what you're trying to do, but my goal was to list all the options:

  1. If its just the nature of logging (stderr VS different system), process.on("unhandledRejection" would handle it. Bluebird does not assume any particular library here but gives you the option to hook to an event.
  2. If its resource management, Bluebird provides tools (using/disposers) to do it safely without the need to crash the process when an unhandled rejection happens. Unlike with domains, resources and state managed with Bluebird.using will always be cleaned up no matter whether a rejection was handled or not. Bluebird assumes this and therefore by default does not crash the process (its not necessary)
  3. If you feel that the above resource management tools are inadequate, you can still attach a handler that shuts down the process.

There is more discussion at #51 and there is a rant describing some of the reasons why I think the defaults are good here

Ah, okay. I can kind of see why Bluebird was designed this way now, especially looking closer at the specifications for ES Promises.

I didn't realise that domains were finally being deprecated so with this knowledge I'm redesigning around using promises as much as possible.

I figured out that unless callback-based functions are promisified, they'll throw exceptions that'll crash the server anyway...and even if a callback-based function is promisified, if it is from a 3rd-party library that I do not control and inside the function they call a callback-based asynchronous function, then that'll also crash the server.

It seems that one must use a catch-all for exceptions even when using promises anyway, as promises cannot catch exceptions that occur within non-promise async functions even when they're wrapped within a promise? This scares me as it is so easy for a developer to use a callback-based function rather than the promisified version, and still crash the server somewhere down the line.

I'll take a gander at your rant. :stuck_out_tongue:

Was this page helpful?
0 / 5 - 0 ratings