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

Fix Non-GET request method with rails exception controller #2317

Merged

Conversation

TonyCTHsu
Copy link
Contributor

What does this PR do?

#2315

When instrumenting a non-GET request, a Rails app with exceptions_app configured would mutated the request method to GET before dispatching to the exception handling app.

https://github.com/rails/rails/blob/8015c2c2cf5c8718449677570f372ceb01318a32/actionpack/lib/action_dispatch/middleware/show_exceptions.rb#L45

Hence, it would be more accurate to fetch the value from the original rack env(prior to the mutation).

@TonyCTHsu TonyCTHsu requested a review from a team October 17, 2022 12:01
@TonyCTHsu TonyCTHsu self-assigned this Oct 17, 2022
@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Oct 17, 2022
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

Looks good! One failing system test (I think it's flaky). Try to re-run and get that green, otherwise, good to merge.

@TonyCTHsu TonyCTHsu merged commit 56d764a into master Oct 17, 2022
@TonyCTHsu TonyCTHsu deleted the fix/th-request-method-with-rails-exception-controller branch October 17, 2022 19:57
@github-actions github-actions bot added this to the 1.6.0 milestone Oct 17, 2022
@TonyCTHsu TonyCTHsu mentioned this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants