Skip to content

Fixes #278: adds support for verbose access logging #279

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 1 commit into from
Closed

Fixes #278: adds support for verbose access logging #279

wants to merge 1 commit into from

Conversation

scottopell
Copy link

@scottopell scottopell commented May 24, 2016 •
edited by thornjad
Loading

Fixes #278

Use -v or --verbose to enable verbose access logging.

Looks like this:

scratch/http-server - [master] » node bin/http-server -p 8000 -v
Starting up http-server, serving ./public
Available on:
  http://127.0.0.1:8000
  http://192.168.0.2:8000
Hit CTRL-C to stop the server
[Tue May 24 2016 12:54:02 GMT-0400 (EDT)] "GET /" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36"
HEADER                    VALUE
host                      localhost:8000
connection                keep-alive
cache-control             max-age=0
accept                    text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
upgrade-insecure-requests 1
user-agent                Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36
accept-encoding           gzip, deflate, sdch
accept-language           en-US,en;q=0.8
if-none-match             "2609389-210-"2016-05-22T20:20:38.000Z""
if-modified-since         Sun, 22 May 2016 20:20:38 GMT

Adds dependency on columnify for pretty printing the headers when enabled.

@scottopell
adds support for verbose access logging
e2e5543 
currently 'verbose' simply means printing out the headers for each
request.
@mwcz

This comment has been minimized.

Sign in to view
@vitalets

This comment has been minimized.

Sign in to view
@vitalets

This comment has been minimized.

Sign in to view
@thornjad
Copy link
Member

I like this, and think it could be useful! There is a conflict, probably because this PR is so old (what's up with that?). Probably an easy fix though since it's not a big change.

@scottopell
Copy link
Author

If there is interest in merging this I can take a look at the merge conflict and fix it. Can't imagine it would be too hard to fix

thornjad
thornjad requested changes Apr 16, 2019
View reviewed changes
bin/http-server
@@ -7,6 +7,7 @@ var colors = require('colors'),
httpServer = require('../lib/http-server'),
portfinder = require('portfinder'),
opener = require('opener'),
columnify = require('columnify'),
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that we either don't add a new dependency, or use it for all logging (and the help output).

Copy link
Member

Choose a reason for hiding this comment

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

I am not a big fan of adding this dependency. It does make the headers look better, but I'm not sure we need the extra baggage to do that. I think util.format should do just fine without any extra libraries

@thornjad thornjad mentioned this pull request Dec 18, 2019
Closed
@thornjad thornjad self-requested a review December 18, 2019 14:48
@thornjad thornjad added this to the v0.13.0 milestone Dec 18, 2019
thornjad
thornjad requested changes Dec 18, 2019
View reviewed changes
bin/http-server
@@ -38,6 +39,7 @@ if (argv.h || argv.help) {
' -K --key Path to ssl key file (default: key.pem).',
'',
' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]',
' -v --verbose Log out the full headers of each request along with the typical content',
Copy link
Member

Choose a reason for hiding this comment

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

We now have -v implemented as the short option for --version, which I think is fine. This option could be just --verbose with no short version.

On the other hand, since http-server is pretty verbose by default, I wonder if this might be better called --very-verbose?

bin/http-server
@@ -7,6 +7,7 @@ var colors = require('colors'),
httpServer = require('../lib/http-server'),
portfinder = require('portfinder'),
opener = require('opener'),
columnify = require('columnify'),
Copy link
Member

Choose a reason for hiding this comment

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

I am not a big fan of adding this dependency. It does make the headers look better, but I'm not sure we need the extra baggage to do that. I think util.format should do just fine without any extra libraries

bin/http-server
@@ -38,6 +39,7 @@ if (argv.h || argv.help) {
' -K --key Path to ssl key file (default: key.pem).',
'',
' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]',
' -v --verbose Log out the full headers of each request along with the typical content',
Copy link
Member

Choose a reason for hiding this comment

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

Could we also move this up next to --silent to keep it all together?

bin/http-server
@@ -38,6 +39,7 @@ if (argv.h || argv.help) {
' -K --key Path to ssl key file (default: key.pem).',
'',
' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]',
' -v --verbose Log out the full headers of each request along with the typical content',
Copy link
Member

@thornjad thornjad Dec 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This also should to be added to the README and doc/http-server.1

@thornjad thornjad added feature:logging minor version non-breaking, non-trivial change labels Dec 18, 2019
@thornjad
Copy link
Member

Closing due to inactivity, please re-open if we'd like to revisit this. I'll leave the associated issue open since it would be nice to have this functionality

@thornjad thornjad closed this Jul 16, 2021
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
4 participants
@scottopell @mwcz @vitalets @thornjad