Skip to content

Pass logger from options to TransportFactory #555

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
Sep 14, 2021

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Sep 13, 2021

I have noticed that a transport error was being logged in https://github.com/getsentry/sentry-php/blob/bfab90c7ef921221b52b75b5108201e87f0f4661/src/Transport/HttpTransport.php#L112-L115

but there always was default NullLogger registered. So I've found that even though I set logger via options it was not being passed to the Transport.

This fixes it.

@ste93cry ste93cry added this to the 4.2 milestone Sep 13, 2021
@simPod simPod force-pushed the pass-logger branch 3 times, most recently from 85afe67 to dad5afb Compare September 14, 2021 08:52
@simPod simPod requested a review from ste93cry September 14, 2021 08:53
Copy link
Contributor

@ste93cry ste93cry left a comment

Choose a reason for hiding this comment

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

Would you mind updating the CHANGELOG too to reference this PR?

@simPod simPod force-pushed the pass-logger branch 2 times, most recently from 9adbc15 to 491daeb Compare September 14, 2021 20:48
@simPod simPod requested a review from ste93cry September 14, 2021 20:53
Copy link
Contributor

@ste93cry ste93cry left a comment

Choose a reason for hiding this comment

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

Everything looks good, thank you for the contribution 💪

@ste93cry ste93cry merged commit 31edf15 into getsentry:master Sep 14, 2021
@simPod simPod deleted the pass-logger branch September 15, 2021 06:54
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.

2 participants