use .isBoom instead of instanceof Boom

[nlf]

For some reason, the existing check was causing a custom authentication plugin to not passing along a status code. I was able to trace it back to this check.

Despite using Boom to create the error, and the isBoom property being set to true, instanceof Boom was false.

Changing this check to use .isBoom instead resulted in the expected behavior.

[nlf]

Not sure why Travis is failing, locally all tests are passing..

689 tests complete (4922 ms)

Coverage: 100.00%
Coverage succeeded

[hueniverse]

I'm not sure checking isBoom alone is safe. We're basically taking over a key that any response object can no longer use. It's ok for now, but I'd like to understand why it wasn't showing instanceof Boom. For example, was the error created with hapi.Error?

[nlf]

here's the plugin I created, sanitized to the smallest possible code, that was giving me grief.

var Boom = require('boom');

exports.register = function (plugin, options, next) {
    plugin.auth('query_user', {
        implementation: {
            authenticate: function (request, callback) {
                if (!request.query.user) return callback(Boom.unauthorized('user is required'));
                var user = request.query.user;
                delete request.query.user;
                return callback(null, user);
            }
        },
        defaultMode: true
    });
    next();
}

The unauthorized error is what was failing to match as instanceof Boom

[hueniverse]

Can you try using hapi.Error instead of Boom without the hapi change? I'm curious if it's a problem of node loading Boom twice.

[geek]

I tried this with hapi 1.8.2 and then installed boom@0.4.0 and was able to reproduce the original issue. If you use Hapi.error instead of using a Boom module with a different version it does work without the PR. What do you all think of reverting this change to force using hapi.error instead of allowing for a different Boom version?

[nlf]

That makes sense to me. Is hapi.Error easily accessible from within a plugin like the above? Or would I still have to require hapi? It seems like it could be useful to hang it off of the plugin object as well if it's not already there.

[geek]

Absolutely, here is the updated code to access error from the plugin:

var plugin = {
    name: 'query_user',
    version: '0.0.1',
    register: function (plugin, options, next) {
        plugin.auth('query_user', {
            implementation: {
                authenticate: function (request, callback) {
                    if (!request.query.user) return callback(plugin.hapi.error.unauthorized('user is required'));
                    var user = request.query.user;
                    delete request.query.user;
                    return callback(null, user);
                }
            },
            defaultMode: true
        });
        next();
    }
};

[nlf]

confirmed that works fine. sounds like this should be reverted and replaced with some sample code instead. thanks guys.

[geek]

Thanks for your help, I will add a better example and documentation around this issue.