-
Notifications
You must be signed in to change notification settings - Fork 194
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
[WIP] Logging #209
[WIP] Logging #209
Conversation
hehe failing in node 4 and 5 after I followed airBnB's eslint - dropping node 4 and 5 support to destructuring assignment or make two small fixes in tests... yaa, remove destructuring - it also sounds so negative. |
@@ -0,0 +1,11 @@ | |||
/* IMPORTANT |
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.
what is this :o
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.
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.
I added a comment in the test about how to generate the file
Ref: #208 |
@jfhbrook What do you think about the idea and about refactoring |
Looks like codecov is broken somehow.. I'll take a quick look |
d666ac7
to
5251181
Compare
Codecov Report
@@ Coverage Diff @@
## master #209 +/- ##
==========================================
- Coverage 75.86% 75.79% -0.08%
==========================================
Files 9 9
Lines 518 533 +15
Branches 117 122 +5
==========================================
+ Hits 393 404 +11
Misses 45 45
- Partials 80 84 +4
Continue to review full report at Codecov.
|
I'm using this branch of ecstatic, to serve svg and image files to another node program and I never see double logging (re-run of |
I know there's code branches that call the original middleware recursively, those would probably be candidates for ways to cause double-logging. I'm down to do some middleware refactors in general, given I have the time and the understanding necessary to make useful ones. This might also touch on #193 #154 #88 |
R. I. P. |
This is my initial idea on how to provide logging for ecstatic.
The PR is only half done but the tests and main output is done.
TODO:
middleware
to be able to log all status codes and response lengthgetNscaDateTime
to handle all time zonesI think this is a candidate for a minor release.
Unfortunately, I don't have time to finish this, this year. If you @jfhbrook or someone else have time, I would love to review.
I noticed that the log code path is run two times for each request, which means that the middleware is run two times for each request but I don't understand why we would do that. The second time the
req.method
isundefined
andreq.url
is encoded.