Error thrown from a mongoose-promise callback function is not caught

Question

I spent too much time trying to figure out why my express.js controller did not respond to a simple query, and figured out that runtime errors fired from a Mongoose-promise callback were silently interrupting the callback process.

Here is a simplified version of my code:

server.get('/api/test', function (req, res, next) {
  User.find({}).exec().then(function success(users){
    console.log('SUCCESS');  
    typo[0] = 1; // throws a runtime error
    res.json(users);
  }, function error(err){
    console.log('ERROR');  
    res.json({error: err});
  });
});

This results in SUCCESS showing up in my console, but nothing happens then. No response is given to the user, the error caused by my typo is not appearing in my console, and the error callback is not called either.

I am aware that one should not throw exceptions from a callback function, but in that case, this was just a typo, and it would make sense to me to be warned (e.g. a stack trace in my standard output) whenever one makes this kind of mistake. (we're humans, after all...)

In your opinion, what's the best way to get feedback whenever this kind of mistakes are made in promise callbacks?

This is the reason I refer to "throw-safe" as "throw-dangerous". The error is caught, invisibly, by the promise code, and "swallowed". Fortunately, the error is also propagated. To make it observable, try adding a `.catch()` to the end of the promise chain, and log the error. — Roamer-1888, Apr 22 '15 at 10:31

Benjamin Gruenbaum · Accepted Answer · 2016-05-02T07:26:28.613

This is Mongoose's fault for using a bad promise implementation. Promises are throw-safe so exceptions are caught (so they can be later handled by future code) - the future code never comes and Mongoose never reports that it did not. Good promise implementations do not suffer from this issue.

Your options are two:

Use a library like Bluebird:

var Promise = require("bluebird");
var mongoose = Promise.promisifyAll(require("mongoose"));

User.findAsync({}).then(function(data){
    JSON.prase("dsa"); // not a silent failure, will show up, easy debugging
});

This has the advantage of being faster than mongoose promises so there is no performance penalty. Alternatively, if you're super conservative and don't want the performance and API gains of bluebird - you can use native promises:

// Promise is the native promise
Promise.resolve(User.find({}).exec()).then(function(data){
    JSON.prase("dsa");
});

And then, assuming you're running a modern variant of nodejs (read: io.js v 1.4.1 or upper), you can subscribe to promise rejections:

process.on("unhandledRejection", function(p, why){
    console.log("FOUND ERROR!!!!", p , why);
});

So exceptions are not silently suppressed.

Thank you for your awesome reply, Benjamin! Too bad that there is apparently no way to use another Promise implementation without replacing (e.g. `findAsync()` instead of `find()`) or wrapping (`Promise.resolve()`) all my Mongoose DB calls... — Adrien Joly, Apr 22 '15 at 16:00
@adrienjoly if it makes you feel any better I asked on the issue tracker too :) if you wanna help open an issue asking them to implement the rejection hooks (it's a spec), maybe they will who knows — Benjamin Gruenbaum, Apr 22 '15 at 16:02
This did not work for me until I added the following: `mongoose.set('error', true)`. My code is slightly different I use `Promise = require("bluebird")` `mongoose = require('mongoose')` `mongoose.Promise = Promise` is there a difference to `Promise.promisifyAll(require("mongoose"))` — Andi Giga, Sep 21 '16 at 18:04

score 3 · Answer 2 · edited Apr 22 '15 at 11:11

3

The exec() has two promises

.then(function) 
.then(null , function)

try this, I think it will help

server.get('/api/test', function(req, res, next) {
    User.find({}).exec()
        .then(function success(users) {
            console.log('SUCCESS');
            typo[0] = 1; // throws a runtime error
            res.json(users);
        })
        .then(null, function error(err) {
            console.log('ERROR');
            res.json({
                error: err
            });
        });
});

edited Apr 22 '15 at 11:11

Bergi

630,263
148
957
1,375

answered Apr 22 '15 at 10:21

hussam

596
5
12

1

No, it does not work. => `TypeError: Object # has no method 'catch'` – Adrien Joly Apr 22 '15 at 10:30
1

sorry i have updated my answer , i had a conflict between mongoose and sequelize in the type of call the promises – hussam Apr 22 '15 at 10:32
1

I believe that `exec().catch(handler)` is similar to `exec().then(null, handler)` in the general case of promises. – Adrien Joly Apr 22 '15 at 10:32
... and using two `then()` is equivalent to combining the success and error callbacks into the same `then()`. cf http://mongoosejs.com/docs/api.html#promise_Promise-then => it does not solve the problem. – Adrien Joly Apr 22 '15 at 10:33
The difference is between `.then(handler).then(null, handler)` and `.then(handler, handler2)` – Benjamin Gruenbaum Apr 22 '15 at 10:47
@adrienjoly: [No, they are not equivalent](http://stackoverflow.com/q/24662289/1048572) - and in this case two `then`s would be appropriate imo. – Bergi Apr 22 '15 at 11:10
@hussam could you tell me in which case it throw an error? I mean if the query return empty, will it enter in .`then(null, handler)` – Marcel Djaman Jun 03 '15 at 15:24

Error thrown from a mongoose-promise callback function is not caught

2 Answers2