Error thrown by middleware should be caught and passed to next()

[sgilroy]

Any uncaught exception in a mongoose pre-save middleware gets ignored and results in the save operation never completing (callback never called or promise never resolved/rejected). For our application, it seems like it would be desirable if mongoose would catch all of these errors and pass them to next() automatically, instead of requiring us to add error handling to every single middleware function. Is there some reason for not implementing such a solution in mongoose?

The example below shows a failing test due to the middleware throwing an error:

var chai = require('chai'),
    expect = chai.expect;
    mongoose = require('mongoose');

describe('mongoose-middleware', function() {
    var MiddlewareErrorModel, doc, preSaveCallback;
    before(function() {
        mongoose.connect('mongodb://localhost/test');

        var MiddlewareErrorSchema = new mongoose.Schema({
            name: String
        });

        MiddlewareErrorSchema.pre('save', function(next) {
            console.log('pre save first middleware');
            preSaveCallback(next);
        });
        MiddlewareErrorSchema.pre('save', function(next) {
            console.log('pre save second middleware');
            next();
        });

        MiddlewareErrorModel = mongoose.model('MiddlewareError', MiddlewareErrorSchema);
    });

    beforeEach(function() {
        doc = new MiddlewareErrorModel({ name: 'MiddlewareError Z' });
    });

    describe('with middleware that throws an error', function() {
        beforeEach(function() {
            preSaveCallback = function(next) {
                throw new Error('example of an error thrown directly by a mongoose save middleware');
            };
        });

        it('should pass the error from the middleware to the callback', function(done) {
            doc.save(function(err, savedDoc) {
                console.log('save callback');
                expect(err).to.be.ok;
                done();
            });
        });
    });
});

Output (tested with mongoose@4.1.12):

  mongoose-middleware
    with middleware that throws an error
pre save first middleware
      1) should pass the error from the middleware to the callback


  0 passing (27ms)
  1 failing

  1) mongoose-middleware with middleware that throws an error should pass the error from the middleware to the callback:
     Uncaught Error: example of an error thrown directly by a mongoose save middleware
      at preSaveCallback (test/test_middleware.js:36:23)
      at model.<anonymous> (test/test_middleware.js:19:13)
      at _next (node_modules/mongoose/node_modules/hooks-fixed/hooks.js:62:30)
      at fnWrapper (node_modules/mongoose/node_modules/hooks-fixed/hooks.js:186:18)
      at model.Object.defineProperty.value.fn (node_modules/mongoose/lib/schema.js:234:9)
      at _next (node_modules/mongoose/node_modules/hooks-fixed/hooks.js:62:30)
      at fnWrapper (node_modules/mongoose/node_modules/hooks-fixed/hooks.js:186:18)
      at node_modules/mongoose/lib/schema.js:217:13
      at complete (node_modules/mongoose/lib/document.js:1111:5)
      at node_modules/mongoose/lib/document.js:1137:20
      at ObjectId.SchemaType.doValidate (node_modules/mongoose/lib/schematype.js:645:22)
      at node_modules/mongoose/lib/document.js:1133:9

[sgilroy]

This is a possible fix I implemented via a patch (using https://github.com/othiym23/shimmer)

'use strict';

var shimmer = require('shimmer');

module.exports = function(mongoose) {
    mongoose = mongoose || require('mongoose');
    var Schema = mongoose.Schema;
    var proto = Schema && Schema.prototype;

    shimmer.wrap(proto, 'pre', function(original) {
        return function() {
            console.log('patching middleware', arguments);
            var parallel = arguments[1];
            var originalCallback = arguments[2];
            if (!originalCallback) {
                originalCallback = arguments[1];
                parallel = false; 
            }
            var callback = function(next, done) {
                console.log('starting patched middleware', arguments);
                done = done || next;
                try {
                    originalCallback.apply(this, arguments)
                } catch(error) {
                    console.log('middleware-patch caught error', error, error.stack);
                    done(error);
                }
            };
            return original.apply(this, [arguments[0], parallel, callback]);
        };
    });
};

[vkarpov15]

@sgilroy sorry for taking a while to get back to you. In general, mongoose tries to defer exception handling to userland where possible, so it'll kill the process if there's an exception in stuff like callbacks or middleware. This definitely seems a little weird in the ES6 paradigm, but IMO it's the right behavior for ES5 code.

[gastonelhordoy]

Hey @vkarpov15, I have just faced this same issue and it took me a while until I realized the error was in a mongoose middleware. TBH I didn't know mongoose would silently kill the process upon any uncaught exception in middlewares. Maybe we could get something printed out in console.error so we have the stack at least before killing the process.

When I realized where the error was, my next thought was why mongoose wouldn't just fail the operation (reject promise in my case but the same would apply to callbacks). Do you still think this wouldn't be the right thing to do? Just curious, what side effect could it cause?

Cheers!

[vkarpov15]

It fails silently? Can you provide a code sample that demonstrates this? As far as I know, mongoose just throws an exception, which means node logs it to the console and exits the process.

[vkarpov15]

Fixed in 5.0 branch 👍

[gastonelhordoy]

Hey, thank you very much!

[SnidelyWhiplash]

@vkarpov15 Is there a recommended way to handle this in 4.x? Can I set an exception handler early in the middleware chain, or..? Thanks.

[vkarpov15]

@SnidelyWhiplash The only way is to just wrap your entire middleware in a try/catch if the middleware might throw a synchronous error. You can use the below wrapper function:

function wrapMongooseMiddleware(fn) {
  return function(next) {
    try {
      fn(next);
    } catch (error) {
      next(error);
    }
  }
}

// Later
Model.pre('save', wrapMongooseMiddleware(function(next) {
  throw new Error('Oops!');
}));

What is preventing you from upgrading to Mongoose 5.x?