search

bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
798 stars 186 forks source link

Option 'statusLevels' in errorLogger #90

Closed Revidend closed 9 years ago

Revidend commented 9 years ago

For the Error Logging an option 'statusLevels' is described as

different HTTP status codes caused log messages to be logged at different levels (info/warn/error), the default is false

The Option is not working for me and I didn't see an implementation in the code of the errorLogger. Is there a plan to implement this? This would be a very nice feature to have and I'm looking forward to use it when implemented.

Many thanks.

floatingLomas commented 9 years ago

It isn't implemented for errorLogger, it's implemented for the regular logger.

It's sort of implied, I think, that anything that makes it to the errorLogger is in fact an error, so I can't imagine why you'd want to label a log event occurring in the error handler as anything other than an error - excepting of course if you're using custom levels, but since we don't expose a way to customize the 'code-level mapping,' that's not really relevant here.

Revidend commented 9 years ago

Our usecase for this feature is, to log 4XX as warnings in a logfile and database. All 5XX errors will additionaly sent to our developers with an sms transport. So in case of a 401 NotAuthorised, cause of wrong password or something else the current developer in duty will not interruped in sleeping.

floatingLomas commented 9 years ago

Sure, fair enough. The 4xx's though won't get through to the error handler unless you're passing them along via next() and then doing your 4xx responses in there.

If you're handling 4xx responses in the router handler itself, the logger should pick them up and mark them as warn as per the statusLevels (and pick up the 5xxs as errors, and do same). As I understand it, you can set a Transport to essentially 'bind' to a level.

If you're handling them by passing something through next(error) to the error handler - which is something that I'd never considered doing - I'd personally say that's outside the scope of our errorLogger, and you might want to consider implementing a regular Winston logger to log the warnings and dispatch the SMS.

@bithavoc, any thoughts?

Revidend commented 9 years ago

Thanks for the hints. I will think about an other solution to solve our requirements.

Maybe the way we are dealing errors is not the best way. We have to consider a better solution for that. In the node environment we're just a little bit half-baked.