Skip to content
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

Log 404s to console #417

Closed
edwardhorsford opened this issue Aug 8, 2017 · 8 comments
Closed

Log 404s to console #417

edwardhorsford opened this issue Aug 8, 2017 · 8 comments

Comments

@edwardhorsford
Copy link
Contributor

The kit currently swallows 404 errors.

We should either not swallow them, or else print them to the console.

Something like console.log('404:' + err) on line 161 of lib/utils.js could work.

@joelanman
Copy link
Contributor

we can probably write a nicer error than the default

@mikeshawdev
Copy link
Contributor

Adding a custom comment would be good! I wonder whether it would also be useful, to add an HTTP request logger, such as Morgan, to the prototype? It would output like below:

morgan-output-example

If folk feel this is, maybe, too much output we could add our own custom log statements in. I think it's useful to see exactly what requests are being served, and the response from the server

@joelanman
Copy link
Contributor

I think the most useful default is to output errors and warnings. If we output everything, as in the screenshot above, it makes it harder to see those. We could make it configurable if people want to see more.

@mikeshawdev
Copy link
Contributor

@joelanman I can add a condition to satisfy that. The log would look like below:

screen shot 2018-01-11 at 11 05 37

It only logs out on status codes that are greater than or equal to 400. Would this resolve the issue?

The other option I had was something like below, which may be easier for various users of the kit to understand but only logs for 404s:

screen shot 2018-01-11 at 11 08 35

@joelanman
Copy link
Contributor

I think a custom message would be good. The first screenshot you have to know what a 404 is.

Not sure 'could not resolve path' is easy to understand either... but it's tricky. I was thinking 'Missing file' - but it's not always a file, could be a route in routes.js. How about

Error: 404 - couldn't find '/url/that/doesnt/work'

As that maps well to the technical meaning of 404 (Not found) but is a bit easier to read

@amyhupe
Copy link

amyhupe commented Jan 11, 2018

Yes to @joelanman's suggestion. Though could you say 'could not' instead of 'couldn't'. I know our current position is that contractions are OK. but I think the proximity to the opening quotation here might make it easier to miss.

@mikeshawdev
Copy link
Contributor

No problem, I'll get this added and create a PR! Will take @amyhupe's suggestion on the "could not" vs "couldn't"

@NickColley
Copy link
Contributor

As referenced in the closed issue, this has now been fixed, thanks everyone.

#566

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants