hapijs / boom

HTTP-friendly error objects
Other
2.94k stars 192 forks source link

boom doesn't consider message not set directly on error object when deciding whether it is a compatible error #246

Closed chrift closed 5 years ago

chrift commented 5 years ago

As a result of the fixes for #216, any error messages set on err.output.payload.error throw the following error:

TypeError: Cannot read property 'configurable' of undefined
    at Object.internals.initialize (.../node_modules/@hapi/boom/lib/index.js:394:27)

It looks like even though on line 385 it checks for an error message stored in err.output.payload.error, it doesn't consider them on line 394.

One possible solution to this could be:

// node_modules/@hapi/boom/lib/index.js:385

    const messageLocation = {
        err,
        message: 'message'
    }

    if (!message &&
        !err.message) {

        err.reformat();
        message = err.output.payload.error;

        messageLocation.err = err.output.payload
        messageLocation.message = 'error'
    }

    if (message) {
        const props = Object.getOwnPropertyDescriptor(messageLocation.err, messageLocation.message) || Object.getOwnPropertyDescriptor(Object.getPrototypeOf(messageLocation.err), messageLocation.message);
        Hoek.assert(props.configurable && !props.get, 'The error is not compatible with boom');

        err.message = message + (err.message ? ': ' + err.message : '');
        err.output.payload.message = err.message;
    }

I can open a PR with this change if desired.

chrift commented 5 years ago

I have opened pr #247 containing style appropriate code from above

hueniverse commented 5 years ago

Can you provide a test case? Just a quick example of how to reproduce this?

chrift commented 5 years ago

Hey Eran,

Here is an example of what led me to find this error:

const Boom = require('./')

class Example extends Error {
  constructor (message) {
    super(message)
    Boom.boomify(this)
  }
}

// console.log(Error())
console.log(new Example())
lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.