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

Add halted_callback to the log output #259

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benoittgt
Copy link

@benoittgt benoittgt commented Sep 10, 2018

Fix #258

Sometimes when you do the final render or redirection, it can be halted by a controller callback. With the existing code, it can very hard to understand where it stopped.

With this commit, the intent is to provide the method name where it stops.

I have only one rubocop offense:

lib/lograge/log_subscriber.rb:8:3: C: Class has too many lines. [102/100]
  class RequestLogSubscriber < ActiveSupport::LogSubscriber ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

32 files inspected, 1 offense detected
RuboCop failed!

What could I do? Extract all logic of extract_runtimes, extract_location, extract_unpermitted_paramsandextract_halted_callbackinto separateclass`?

Sometimes when you do the final render or redirection, it can be
halted by a controller callback. With the existing code, it can
very hard to understand where it stopped.

With this commit, the intent is to provide the method name where
it stops.
@benoittgt
Copy link
Author

We are using this on production since the PR without any issue.

@iloveitaly
Copy link
Collaborator

@benoittgt could you rebase on master so CI runs?

@jeremybdk
Copy link

jeremybdk commented Sep 12, 2023

@benoittgt would be great if you could rebase on master if you have time, thank you!

Sorry, actually I ended up doing it myself here #373

For now it won't work for every case so need to figure this out first, and merging this PR is not yet possible

@iloveitaly
Copy link
Collaborator

@jeremybdk could you submit a new PR which doesn't have a master conflict?

@benoittgt
Copy link
Author

This should do it. #375

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 this pull request may close these issues.

How to log the before_action method that halt the request?
3 participants