Change set status assert, use statuses when possible #1308
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1308 +/- ##
======================================
Coverage 100% 100%
======================================
Files 5 5
Lines 393 393
======================================
Hits 393 393
Continue to review full report at Codecov.
|
fl0w
force-pushed
the
change-assert-status
branch
from
24b6bf8
to
6d7e949
Compare
|
Forced pushed an update to further constrain expression. |
lib/response.js
Outdated
| @@ -84,7 +84,7 @@ module.exports = { | |||
| if (this.headerSent) return; | |||
|
|
|||
| assert('number' == typeof code, 'status code must be a number'); | |||
| assert(statuses[code], `invalid status code: ${code}`); | |||
| assert(/^[0-9]{3}$/.test(code), `invalid status code: ${code}`); | |||
There was a problem hiding this comment.
how about
assert(Number.isIngeter(code) && code >= 100 && code <= 999, 'message')There was a problem hiding this comment.
Yea, I'll update it. I ran a quick bench just to check how bad RegExp is in comparison (even though it's pointless in a real world scenario as I can't imagine ctx.status being a bottleneck:
Valid RegExp x 37,061,865 ops/sec ±0.51% (88 runs sampled)
Valid expression x 728,862,990 ops/sec ±1.60% (82 runs sampled)The only thing I'd change is to not check both for number and Number.isInteger. I'll move isInteger to the above assert and keep that message (less "breaking") and replace RegExp.
fl0w
force-pushed
the
change-assert-status
branch
from
6d7e949
to
65c000a
Compare
|
Updated, squashed and rebased. |
|
koa@2.7.0 released |
Reference, #1119 #1042
This is a different approach to #1119 for allowing custom statuses. Instead of adding an API in Koa on top of statuses, just don't disallow anything unless it's a 3 digit integer.
This is a result from the fact that #1119 got stale or never got any traction, that statuses package can be
repopulatedadded to by its design, dead-horses comment about RFC and no real purpose to disallow custom statuses, and the fact that I personally dislike previous approaches.