1
\$\begingroup\$

I have multiple controllers and every controller has this error handler:

if (error) {
  res.render('error', {
     error: error
  });
}

Sometimes I have more than one:

exports.update = function(req, res, next) {
    MyModel.findById(req.params.id, function(error, event) {
        if (error) {
            res.render('error', {
                error: error
            });
        }
        else {
            event['active'] === false ? event['active'] = true : event['active'] = false;

            event.update({
                'active': event['active']
            }, function(error) {
                if (error) {
                    res.render('error', {
                        error: error
                    });
                }
                else {
                    res.end('Success!');
                }
            });
        }
    });
};

I have more than 30 controllers and this seems like an inefficient way of handling.

\$\endgroup\$
1
\$\begingroup\$

If all of your functions adhere to this pattern you can wrap them in, you guessed it, another function:

defaultError = function(handler) {
    return function(error) {
        if (error) {
            res.render('error', {
                error: error
            });
        }
        else {
            return handler.apply(null, Array.prototype.slice.call(arguments, 1));
        }
    };
};

This uses the special arguments object, which has to be converted to an array before you can slice off the error object, and apply on the passed-in function.

Now the previously repetitive code gets a bit shorter:

exports.update = function(req, res, next) {
    MyModel.findById(req.params.id, defaultError(function(event) {
        event['active'] === false ? event['active'] = true : event['active'] = false;

        event.update({
            'active': event['active']
        }, defaultError(function() {
            res.end('Success!');
        }));
    }));
};

Now obviously the defaultError can also customize the rendered error, so as long as error is in a fixed position in the call, this pattern can be used to build a small control flow abstraction.

The cost here is a possible speed penalty due to the manipulation of arguments and the apply call.

If you had promises a generic error handler would be equally viable, but it looks like that is not an option.

\$\endgroup\$
0
\$\begingroup\$

One possible solution would be to promisify your code using one of promise libs like Kris koval's Q or Bluebird and use default error handling function in catch block.

\$\endgroup\$
1
  • \$\begingroup\$ Hi. Welcome to Code Review! We tend to favor longer answers that include examples of the code changes that you are proposing as well as explanations of why the changes are improvements. \$\endgroup\$ – Brythan Feb 7 '15 at 19:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy