Remove inherits and setprototypeof dependenciess, use real classes

[justinfagnani]

Fixes #110 and #98

This converts the HttpError constructor function to a real class and the constructor factories to use subclasses. This removes the need for inherits. setprototypeof wasn't needed anymore with that change.

Changing to real classes fixes #98 as HttpError is now subclassable. The ban against direct instantiation of HttpError is re-implemented with a new.target check.

Also, Error.captureStackTrace should no longer be necessary as the Error constructor captures the stack up to the Error subclass's constructor.

[wesleytodd]

Hey, this is for sure the direction we had planned on going. I think there is also #105 which was started by @jonchurch. I will wait to see what he says, but want to add that until we get a new major out of this (which I am not sure when we plan to), the package supports back to 0.8 and so cannot accept this as is.

Edit: the way I phrased that might have not completely expressed my meaning, sorry. I should add here that we are absolutely looking for folks to help, and if you are willing to help us with more than just this to get it to the next major release we would happily have the help.

[justinfagnani]

@wesleytodd sounds good, was just sending this out to make sure classes could be used with all tests passing.

With a major version, I think a lot more things could be simplified too.

[wesleytodd]

I think what we will need to do for this is start a 3.x branch and re-target to that branch and then also in there start updating the CI (drop everything before current LTS) and what not to prepare for cleaning this stuff up.

@jonchurch I know you added yourself as a repo captian, and if you are interested in driving this release let me know. I only added myself originally because I wasn't sure anyone else wanted to take this one over, and will happily let you run this one if that is what you want. Otherwise I can do the above steps, I will await your answer before taking action.

[justinfagnani]

@wesleytodd and @jonchurch is there any good discussion about the type and extent of changes that would be considered?

I got interested here because I considered using this library in a server project of mine, and what I ended up using is something of my own that I think is a lot simpler - essentially just a HttpError extension of Error with the most of fields from this package, then I might do codegen for some error-code specific subclasses.

That would be ~63 subclass declarations in source rather than dynamically generating them, with the possibility of keeping createError() to dispatch to the right constructor. The source would be very simple, and easy for type declarations.

[wesleytodd]

I am not overly opinionated here, the main thing I care about is the UX improvements to be made by using sub-classable (is that a real word? 😆) internals. Honestly I see this lib as a low change velocity package so once the initial work is done it shouldn't change that much, hence why I am alright not trying overly hard to improve the maintainer experience here. That said, I am not opposed to it either. So if you want to do the work then I would say I am open to the changes you propose.

[codenomnom]

@justinfagnani hey there 👋 Good job on the PR! Do you have your code that you mentioned pushed somewhere? I'm interested in something simpler, and I'm tired of reinventing the wheel in each project 😊 Thanks!

[chyzwar]

Maybe is worth doing new major of http-errors, since express.js v5 will be soon released ?

[wesleytodd]

Express 5 has already been released, and we cut out anything that was not a breaking change for express. I am pretty sure this change would be a major here but not in how we use it in express. For example here: https://github.com/expressjs/express/blob/b31910c542c7079d8a763aff346400b6f4c0eaee/lib/response.js#L17

Thus, we can land it any time and do a major here if we like. The team has just be VERY focused on security and the v5 release lately, so sorry for the delay here. This one is on my radar though, and as we get time to start reviewing and preparing release again we will get to it.

[mbtools]

Removing the two dependencies saves ~420 GB of download bandwidth per week (55 mill * (4 + 4) KB). Go for it 👍