Arbitrary properties on new createError[code || name]([msg]))?

[olalonde]

Would be useful if we could pass arbitrary properties to errors using constructors. e.g.:

new createError[code || name]([msg],[properties]))

this would match the createError function signature. For example, right now we can do:

  createError(402, 'Your balance is too low.', {
    detail: {
      currentBalance: 100,
      price: 150,
    },
  })

but not

  new createError.PaymentRequired('Your balance is too low.', {
    detail: {
      currentBalance: 100,
      price: 150,
    },
  })

[dougwilson]

Hi @olalonde, so to provide some background on that to start this discussion, all those functions are exposed mainly so you can use instanceof on errors to figure out what type of error they are, while the main factory function is your destination to actually do error construction, not the error class functions themselves. Because of this, the constructor to the classes only just matched that of all other native Error objects in JavaScript: it only accepts a single message string argument.

Of course one could easily use Object.assign to add whatever properties to the new object all in one line, and the factory allowing the mixin is really just a stop-gap before JavaScript got around to adding Object.assign in the first place.

But this is not to say this mixin behavior is useful, though I think it should actually not be a generic mixin (that is what Object.assign, part of the JavaScript language, does, and there is no reason we should be re-inventing parts of the language over and over). Better yet, as part of something like #27 we can add this object that is no longer a generic object you would use with Object.assign, but rather actual options to the constructor, where it only accepts things that have meaning.

Anyway, this is a good jumping off point to a lengthy discussion on this topic, thanks for bringing it up, @olalonde!

[olalonde]

Got it. IMO, it wouldn't be that big of a deal if http-errors constructor took an additional argument, they'd still be a superset of Javascript errors.

e.g. (in node v6):

$ node
> class SomeError extends Error {
... constructor(msg, props) {
..... super(msg)
..... Object.assign(this, props)
..... }
... }
[Function: SomeError]
> err = new SomeError('some msg', { foo: 'bar' })
{ Error: some msg
    at SomeError (repl:3:1)
    at repl:1:7
    at REPLServer.defaultEval (repl.js:272:27)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:441:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
    at REPLServer.Interface._onLine (readline.js:219:10)
    at REPLServer.Interface._line (readline.js:561:8) foo: 'bar' }
> err.foo
'bar'
> err instanceof Error
true
> err instanceof SomeError
true
>

On the other hand, Object.assign(new createError.PaymentRequired('balance too low'), { foo: 'bar' }) is not that much more typing then new createError.PaymentRequired('balance too low', { foo: 'bar' }), but still I don't see why this library should limit itself to a single argument as long as it's optional. TBH, Javascript errors kind of suck.. :)

[dougwilson]

Hi @olalonde, yes, there is no reason it cannot take more, I was just giving the history of why it is the way it is now. Do you have any comments on what I'm suggesting the functionality of the second argument should be?

[dougwilson]

I'm going to close this issue, since my last question was never answered and no other discussion has happened around this topic in all the months it has been open.

[mshwery]

This issue may not be worth re-opening, but I'll chime in regardless.

I think it would be generally good to have the method signatures be consistent between createError and createError[code || name].

In my case I find it more ideal and semantically clear to use createError.BadRequest(...) than createError(400, ...), but I lose out on additional properties like expose which would be very useful for sensitive errors in specific situations (probably not 400s though).

[skbolton]

I agree with @mshwery and others I assumed at first glance that you could pass additional props to the helper functions. Not a deal breaker or anything but I would be interested in the features being discussed more

[dhessler]

+1 for a consistent interface.

In my use case I would like to use the expose flag to determine if the error message should be exposed to the end-client. In some cases (e.g., auth) the 4xx client errors messages are sensitive so I'd like to return an ambiguous message in the case that the http error doesn't have the expose flag set.

[dougwilson]

Definitely! Any thoughts on my proposal / thoughts above?

[dougwilson]

Closing again as stale.

[jwater7]

Ah, thanks - I see this is a common unaddressed request. Thanks for the link - hope this gets implemented someday, but unless you keep a ticket open somewhere this issue will probably not get any attention and people will keep stumbling over the interface trying to get it to work until they re-read the docs and realize that it's not implemented (like I did). At the very least, maybe make a nice bold note of it in the README.

[AbdelrahmanHafez]

I'm not sure why we'd allow attaching properties using createError but not createError.Unauthorized.

It's clear that we're trying to match JS error patterns, but there I'd argue that consistency among both ways of constructing an error should be more important than strictly allowing one parameter only.

I'm preferring new HttpErrors.PaymentRequired over createError(402) because it won't require someone reading the code to know that 402 means PaymentRequired.

Would happily put in a PR that allows that functionality.

[dougwilson]

I find it strange that this issue keeps getting comments, yet no one is actually replying to the proposal I gave. These comments are not very constructive by simply replying without considering the actual conversion in the issue.

It turns out, Javascript has now added an option object to Error constructors, and actually follows the proposal I laied out above. The next major revision will be following the Javascript Error object standard for the options argument to Error constructors and the code changes are already underway, with 2.0 release imment. No need to create any additioal PRs and duplicate work around this issue.