Skip to content

Make the RequestIntegration integration fetch the request from the request stack #361

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

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Make the RequestIntegration integration fetch the request from the request stack #361

merged 1 commit into from
Jan 4, 2021

Conversation

ste93cry
Copy link
Contributor

@ste93cry ste93cry commented Aug 7, 2020

With this PR the RequestIntegration integration now fetches the HTTP request from the request stack of Symfony instead of creating a new request from the PHP superglobals. This should (but I didn't test if it was really broken before) also solve the fact that in the context of a Symfony sub-request the request data attached to the event was referring to the main PHP request

@ste93cry ste93cry requested a review from Jean85 August 7, 2020 21:59
Copy link
Contributor

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

PR looks fine apart from a small comment. You should retarget/rebase to the 3.x branch though, master is for the future 4.0.

@Jean85
Copy link
Contributor

Jean85 commented Dec 22, 2020

@ste93cry should we rebase and merge this?

@ste93cry
Copy link
Contributor Author

Yes, I will do it after a few other pending PRs get merged

@ste93cry ste93cry requested a review from Jean85 January 2, 2021 18:23
@Jean85 Jean85 merged commit 40ff42b into getsentry:master Jan 4, 2021
@ste93cry ste93cry deleted the feature/request-integration-symfony-based-request-fetcher branch January 4, 2021 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants