deps: update http-errors to v2.0.0 by 3imed-jaberi · Pull Request #1486 · koajs/koa (original) (raw)

@3imed-jaberi

@codecov

dead-horse

@3imed-jaberi

@dead-horse,
I liked the latest updates to be present in the package.json file as you see, but if there's a problem with that I will close all updates PRs except for #1480 😇.

@miwnwski

I will close all updates PRs except for #1480 😇.

Would you mind closing all minor/patch PRs except or should I do that for you?

@3imed-jaberi

I will close all updates PRs except for #1480 😇.

Would you mind closing all minor/patch PRs except or should I do that for you?

Thank u @miwnwski, i will do. please can you tell me what you think here #1480

This was referenced

Aug 19, 2020

@miwnwski

please can you tell me what you think here #1480

I'm not to sure what to say about it, it's a breaking change so that can't be merged unless we do a major bump. Also, as dead-horse mentioned, I agree that we need better approach to handle to custom statuses in Koa.

Statuses could have an API for adding and changing statuses, however I feel that's out of @jshttp/statuses scope. Also, it's just an object which can be modified so "an API" might just be over-engineering it. Simply exporting statuses in Koa lib feels inelegant/lazy IMO.

I personally feel that a @koajs/statuses should be created (which in turn depends on @jshttp/statuses) that Koa should depend on instead. That would allow for some extra convenience, but keeps complexity where it belongs.

Others might disagree.

@dougwilson

To add to this (I'm typically the person around various things like http-errors):

  1. I am happy to perform some kind of critical patch against the previous major version of a module like http-errors if needed.
  2. There has definately been discussions around Koa regarding custom statuses and how to manage them better. What exists now (and thus breaks with the http-errors upgrade) is kind of a best effort anyway, as changing the data in statuses module is just a best hope that http-errors and others actually have the same copy of it in the node_modules tree in order for it to work.

There was a discussion / idea about adding an API to statuses, but as @miwnwski mentioned it is likely out of scope -- besides still having the general issue where you're still hoping that the statuses you're changing + the statuses http-errors uses is the same copy in node_modules in the current design.

I believe a lot of this (correct me if I'm wrong) has to do with Koa rejecting replying with a status it does not know about, so folks certainly need to add custom statuses for that case as the most common on. I'm not saying that is good or bad, just I think that is the main source for the custom statuses management in Koa.

Since http-errors will accept any error status code, even if it does not exist in statuses, I think there could be some kind of overall status management in Koa (like suggested by @miwnwski ) that ties all this together.

@miwnwski

@dougwilson thank you, I was hesitant to ping you regarding my thoughts on statuses because it felt like bikeshedding.

I believe a lot of this (correct me if I'm wrong) has to do with Koa rejecting replying with a status it does not know about, so folks certainly need to add custom statuses for that case as the most common on. I'm not saying that is good or bad, just I think that is the main source for the custom statuses management in Koa.

That's an interesting observation. With that in mind, one could imagine a flag to force or not the existence of a status code. But I still think that's an implementation that probably lives better in separate Koa-centric statuses package.

I'll draft up a PR and a repo on Sunday and ping you for feedback unless somebody beats me to it.

3imed-jaberi

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to the latest release v2.0.0 ✅

3imed-jaberi

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3imed-jaberi 3imed-jaberi changed the titledeps: update http-errors to v1.8.0 deps: update http-errors to v2.0.0

Apr 2, 2022

etroynov

@etroynov

@3imed-jaberi

3imed-jaberi

Comment on lines -35 to -49

describe('ctx.throw(err, status)', () => {
it('should throw the error and set .status', () => {
const ctx = context()
const error = new Error('test')
try {
ctx.throw(error, 422)
} catch (err) {
assert.strictEqual(err.status, 422)
assert.strictEqual(err.message, 'test')
assert.strictEqual(err.expose, true)
}
})
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanong

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})