bugsnag / bugsnag-symfony

BugSnag notifier for the Symfony PHP framework. Monitor and report errors in your Symfony apps.
https://docs.bugsnag.com/platforms/php/symfony
MIT License
43 stars 21 forks source link

fix: checks haystack is null before using stripos() #147

Closed niamzor closed 2 years ago

niamzor commented 2 years ago

I got a deprecated error when using bugsnag-symfony with PHP 8.1 :

stripos(): Passing null to parameter #1 ($haystack) of type string is deprecated in vendor/bugsnag/bugsnag-symfony/Request/SymfonyRequest.php line 104

This is my first PR, so please tell me if something is not right.

Goal

Make bugsnag-symfony compatible with PHP 8.1.

Changeset

It checks that $haystack is not null before using stripos()

Testing

I ran phpunit tests

luke-belton commented 2 years ago

Hi @phillylovepark - thanks for the PR! We're looking to add a static analysis tool like psalm/phpstan to bugsnag-symfony at some point in the future. This should help fix deprecated error messages such as this. We'll review this PR as part of that work when priorities allow 👍

tarasTrujay commented 2 years ago

Hey @luke-belton! I really need this PR

luke-belton commented 2 years ago

hey @tarasTrujay - as this is a deprecation warning, it's not considered a critical fix for us at this time while we have other higher priority work on our backlog. I don't have an exact timescale for the static analysis changes I described in my comment above, however I will notify this thread with any updates. The deprecated change being warned about won't break until php 9.0 is released, which is still a long way off.

tarasTrujay commented 2 years ago

@luke-belton But you can merge this PR and that's it

imjoehaines commented 2 years ago

Thanks for the PR @phillylovepark! I'm going to merge this down in https://github.com/bugsnag/bugsnag-symfony/pull/149 where I've added some unit tests for this class