Add support for custom statuses #1119
Conversation
LukeBousfield
force-pushed
the
custom-statuses
branch
from
8966c35
to
9048354
Compare
Codecov Report
@@ Coverage Diff @@
## master #1119 +/- ##
==========================================
+ Coverage 99.73% 99.73% +<.01%
==========================================
Files 5 5
Lines 372 375 +3
==========================================
+ Hits 371 374 +3
Misses 1 1
Continue to review full report at Codecov.
|
| */ | ||
|
|
||
| addStatus(code, description) { | ||
| this.customStatuses[code] = description; |
There was a problem hiding this comment.
only issue is naming. i'd prefer this to be called customStatusCodes. what does everyone else think?
There was a problem hiding this comment.
other than that, i'm good with this PR
There was a problem hiding this comment.
Not that my opinion matters, but personally I'd prefer addCustomStatusCode, erring on the side of a more descriptive name
| @@ -46,6 +46,18 @@ module.exports = class Application extends Emitter { | |||
| this.context = Object.create(context); | |||
| this.request = Object.create(request); | |||
| this.response = Object.create(response); | |||
| this.customStatuses = {}; | |||
| } | ||
|
|
||
| /** | ||
| * Add custom status code |
There was a problem hiding this comment.
All other comments have a period after the description text.
|
Any updates on merging this fix? We've run in to the same problem and trying to monkey patch has introduced other issues. |
|
Highly subjective and not a reason for this to not being merged I dislike this solution. I would much rather see statuses provide this and Koa simply delegating. Not sure this is of interest to jshttp though. |
| assert(!this.res.headersSent, 'headers have already been sent'); | ||
| this._explicitStatus = true; | ||
| this.res.statusCode = code; | ||
| if (this.req.httpVersionMajor < 2) this.res.statusMessage = statuses[code]; | ||
| if (this.req.httpVersionMajor < 2) this.res.statusMessage = statuses[code] || this.customStatuses[code]; |
There was a problem hiding this comment.
If I get you correctly, you meant to use the custom status codes instead of the default status codes.Is that right? So, actually the logic would be:
this.customStatuses[code] || statuses[code];
Then, if you have no custom status codes the default status will be used instead. Is that the expected behavior?
|
@fl0w Hey man, I don't understand why this has not been addressed yet. The way I see it, KOA has to either provide a mechanism or go with a different status module because as one of the maintainers of They are leaning towards the former (being a list of known status codes), and it sounds like (and is apparent by the code) that KOA is leaning towards the latter. So if KOA is leaning towards the latter, it should provide a mechanism instead of imposing on the |
|
@fl0w , @jonathanong , @MarkHerhold , @chittolina and @CodingPower472 I am facing the same issue with http status codes that are not present in Koa, any updates on this feature please |
|
closes via #1308 |
Fixes #1042