spatie / laravel-ray

Debug with Ray to fix problems faster in Laravel apps
https://myray.app
MIT License
291 stars 64 forks source link

Added DeprecatedNoticeWatcher that piggy backs off of the Application… #229

Closed JuanRangel closed 2 years ago

JuanRangel commented 2 years ago

This PR adds a calls the (new DeprecatedNoticeWatcher())->concernsDeprecatedNotice($message) method from within the ApplicationLogWatcher. The deprecated log notices are disabled by default.

Adding SEND_DEPRECATED_NOTICES_TO_RAY=true in the .env file will enable the notices to go out to ray again.

freekmurze commented 2 years ago

Thanks for your work on this.

Right now, all messages that contain the word "deprecated", even on the normal channel, will be ignored.

Can you test if a incoming message is coming from the deprecated channel? That way, messages with the word "deprecated" on other channels will still pass.

Also, could you take a look at the failing tests?

JuanRangel commented 2 years ago

Unfortunately I cannot find a way to filter down to the channel. The log is written without a context and by the time we are able to listen for it in laravel ray using the MessageLogged event we only have the level and message.

If anyone has any idea on how to handle this, feel free to comment.

freekmurze commented 2 years ago

A lot of people will start seeing deprecation warnings when upgrading to PHP 8.1, so I've decided to already go with this way of going about things, until we find a better way.

Currently the tests are failing. Could you make them pass in each environment.

When all tests are green, I'll merge this in.

Thanks for your work on this.

JuanRangel commented 2 years ago

So the problem with the tests was the trigger_error method was causing the tests to fail in the GitHub actions. I replaced the trigger_error with Log::warning. This is actually what Laravel does in the HandleExceptions::handleDeprecation method which is causing the error.

JuanRangel commented 2 years ago

Looks like the RayTest class was importing the incorrect Log (doesn't exist). I imported the Log Facade instead as there are several tests that use it.

freekmurze commented 2 years ago

Seems like we have failures because of deprecation warnings in Laravel 7. Do you know how to let PHPUnit pass even though there are deprecation warnings?

JuanRangel commented 2 years ago

I doesn't seem possible. I went down the google rabbit hole to find a solution, but no luck. I was finally able to get the failure to happen locally, but I am unable to get the tests to pass.

freekmurze commented 2 years ago

Could you try adding this line to the setup method of the base test to see if that helps?

error_reporting(E_ALL & ~E_STRICT & ~E_DEPRECATED);
JuanRangel commented 2 years ago

@freekmurze It didn't work. Same error During inheritance of ArrayAccess: Uncaught ErrorException: Return type of Illuminate\Http\Request::offsetExists($offset)

freekmurze commented 2 years ago

Try adding this: PHPUnit_Framework_Error_Deprecated::$enabled = false; (hat tip to @lukeraymonddowning)

freekmurze commented 2 years ago

Ok, thanks for trying, I'll just ignore the P8.1 / Laravel 7 combination in the workflow. 👍