-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
better demonstrate middleware flow #1195
Conversation
It's not incredibly clear from this example that middleware can act on context changes from other middleware. This updates the logger middleware to use the calculated response time value from the response time middleware instead of calculating it twice.
Codecov Report
@@ Coverage Diff @@
## master #1195 +/- ##
=======================================
Coverage 99.73% 99.73%
=======================================
Files 5 5
Lines 374 374
=======================================
Hits 373 373
Misses 1 1 Continue to review full report at Codecov.
|
I'm +/-0 on this. |
I get where you're headed on this, and I dig it, but wouldn't this make the |
@doug-wade measured in milliseconds in this simple of an example I'd be surprised if there was a difference. If there was a difference, we are already logging one value and sending back another so I'd imagine it's a non-issue. |
const ms = Date.now() - start; | ||
ctx.set('X-Response-Time', `${ms}ms`); | ||
const rt = ctx.response.get('X-Response-Time'); | ||
console.log(`${ctx.method} ${ctx.url} - ${rt}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not good here because rt
is a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dead-horse I'm not sure what you mean; can you elaborate please? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mind, my misunderstanding.
The way this example is written makes it seem like middleware can't act on the context changes of other middleware. This updates the logger middleware to use the calculated response time value from the response time middleware instead of calculating it itself.