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

Feature/rails error handling improved #108

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MarcGrimme
Copy link
Collaborator

@MarcGrimme MarcGrimme commented Jun 25, 2016

Background: Currently mostly all errors happing in the app that should be caught by the rack middleware are logged as normal text output which is wrong. Expected would be some json.

This PR adds a middleware ::Logstasher::ActionDispatch::DebugExceptions to the stack that will just do the logging. This middleware will just sit on top of ::ActionDispatch.:DebugExceptions.

If suppress_app_logs is switched on it also needs to silence the action_dispatch.logger. This is done by replacing it with the ::LogStasher::NullLogger another helper class to make this happen. This part I'm not so proud of cause it should not be necessary but after a long time of struggling I think it is the best I can come up with.

So have fun testing.

Comments welcome.

Try to fix #79

@MarcGrimme MarcGrimme added the bug label Jun 25, 2016
@MarcGrimme MarcGrimme changed the title Feature/rails error handling improved [WIP] Feature/rails error handling improved Jun 25, 2016
@MarcGrimme MarcGrimme force-pushed the feature/rails-error-handling-improved branch 2 times, most recently from 04ed8b2 to 2738115 Compare June 25, 2016 09:57
config.after_initialize do
initializer :logstasher do |app|
app.config.middleware.use ::LogStasher::ActionDispatch::DebugExceptions
app.config.middleware.delete ::ActionDispatch::DebugExceptions if Rails.env.production?
Copy link

@sfate sfate Jun 26, 2016

Choose a reason for hiding this comment

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

Why do not use this condition on :use method?
E.g.

app.config.middleware.use ::LogStasher::ActionDispatch::DebugExceptions unless Rails.env.production?

@MarcGrimme MarcGrimme force-pushed the feature/rails-error-handling-improved branch 2 times, most recently from 1576108 to dc69286 Compare June 30, 2016 13:17
@MarcGrimme MarcGrimme changed the title [WIP] Feature/rails error handling improved Feature/rails error handling improved Jul 3, 2016
@MarcGrimme MarcGrimme force-pushed the feature/rails-error-handling-improved branch from 2ca49ea to 67e51e6 Compare July 6, 2016 16:19
@MarcGrimme MarcGrimme force-pushed the feature/rails-error-handling-improved branch from 67e51e6 to 9e089db Compare July 6, 2016 16:26
…::DebugExceptions so that it can be used outside as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logstasher is not Logging Errors
2 participants