Skip to content

add super verbose, show unknown income request header and body #493

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

add super verbose, show unknown income request header and body #493

wants to merge 7 commits into from

Conversation

foxundermoon
Copy link

@foxundermoon foxundermoon commented Jan 27, 2019 •
edited by thornjad
Loading

Fixes #278

@thornjad thornjad self-requested a review February 28, 2019 14:26
thornjad
thornjad requested changes Dec 13, 2019
View reviewed changes
Copy link
Member

@thornjad thornjad left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure "superverbose" is the best term to use here. Many CLI tools have levels of verbosity, and the usual naming would be very-verbose. What do you think of calling it that?

Looks like there are also some merge conflicts to take a look at.

@thornjad thornjad added feature:logging minor version non-breaking, non-trivial change labels Dec 13, 2019
@thornjad thornjad mentioned this pull request Dec 18, 2019
Closed
thornjad
thornjad requested changes Dec 18, 2019
View reviewed changes
Copy link
Member

@thornjad thornjad left a comment

Choose a reason for hiding this comment

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

Had a couple more thoughts while I was looking at #279

bin/http-server Outdated
}
logger.info(colors.cyan(headerStr));
}
if (req.readable) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of logging the request body, I think it might be a bit too verbose. What do you think?

bin/http-server
info: console.log,
request: function (req, res, error) {
var date = utc ? new Date().toUTCString() : new Date();
if (error) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we be showing all this info if there was an error, or if there was no error? Was the logic accidentally flipped here?

@thornjad thornjad added this to the v0.13.0 milestone Dec 18, 2019
foxundermoon and others added 6 commits December 24, 2019 16:45
@foxundermoon @thornjad
Apply suggestions from code review
4d8b21d 
Co-Authored-By: Jade Michael Thornton <jade@jmthornton.net>
@foxundermoon @thornjad
Apply suggestions from code review
ccf67a9 
Co-Authored-By: Jade Michael Thornton <jade@jmthornton.net>
@foxundermoon @thornjad
Apply suggestions from code review
2f51f2c 
Co-Authored-By: Jade Michael Thornton <jade@jmthornton.net>
@foxundermoon
Merge remote-tracking branch 'master'
1b178b8
@foxundermoon
fix format
a84c164
@foxundermoon
add suggested
db3e576
@thornjad thornjad mentioned this pull request Oct 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thornjad thornjad thornjad requested changes

Assignees
No one assigned
Labels
minor version non-breaking, non-trivial change
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.

Add --verbose to log out the headers of each request
2 participants
@foxundermoon @thornjad