search

rollbar / rollbar-php-laravel

Rollbar error monitoring integration for Laravel projects
https://docs.rollbar.com/docs/laravel
140 stars 39 forks source link

add Laravel 10 support #140

Closed jonnott closed 1 year ago

jonnott commented 1 year ago

Description of the change

Add Laravel 10 support

Type of change

Maintenance

jonnott commented 1 year ago

@cyrusradfar @danielmorell .. or anyone currently at Rollbar .. can you hear us?

jonnott commented 1 year ago

Thanks @danielmorell .. how to we move this to getting a new release tagged, seeing as Laravel 10.0.0 is now released?

michaelscola commented 1 year ago

I'm not sure how this would be fully working, as Laravel 10 upgrades to Monolog 3 and the current installed version only does Monolog 2. I will be doing a pull request later for both rollbar-php-laravel and rollbar-bar php to address this.

jonnott commented 1 year ago

I'm not sure how this would be fully working, as Laravel 10 upgrades to Monolog 3 and the current installed version only does Monolog 2. I will be doing a pull request later for both rollbar-php-laravel and rollbar-bar php to address this.

@michaelscola Do you mean a bit like this PR? https://github.com/rollbar/rollbar-php/pull/600

michaelscola commented 1 year ago

I'm not sure how this would be fully working, as Laravel 10 upgrades to Monolog 3 and the current installed version only does Monolog 2. I will be doing a pull request later for both rollbar-php-laravel and rollbar-bar php to address this.

@michaelscola Do you mean a bit like this PR? rollbar/rollbar-php#600

@jonnott I do mean like that PR, except the Monolog handler in the rollbar-php-laravel package is not going to work properly when Monolog 3 is installed. The write method has an updated typehint for LogRecord instead of array.

jonnott commented 1 year ago

@jonnott I do mean like that PR, except the Monolog handler in the rollbar-php-laravel package is not going to work properly when Monolog 3 is installed. The write method has an updated typehint for LogRecord instead of array.

Please do add further commits/PR to resolve that problem.

jonnott commented 1 year ago

@danielmorell I think this PR will need to be re-opened and be added to - it'll need a new major version of this package (BC break) and possibly also a new major version of the underlying rollbar-php package.

Laravel 10 requires monolog v3, which rollbar-php doesn't support (it only supports v2), and also as @michaelscola points out, there's a type change to the argument on the monolog/RollbarHandler.php write() method requiring a Monolog\LogRecord instance.

michaelscola commented 1 year ago

@jonott this absolutely is a breaking change, I'm currently testing the change myself in development and I will add a new pull request once I can confirm everything is working correctly.

jonnott commented 1 year ago

@jonott this absolutely is a breaking change, I'm currently testing the change myself in development and I will add a new pull request once I can confirm everything is working correctly.

Looking forward to your PR. I'm assuming you'll also have to PR a breaking change to https://github.com/rollbar/rollbar-php ?

danielmorell commented 1 year ago

Thank you for pointing out the monolog v3 conflict!

I am just finishing up upgrading https://github.com/rollbar/rollbar-php to support monolog v3. The last part has been updating the test suite. I should have a PR out tomorrow. I will tag this thread once I do.

jonnott commented 1 year ago

Thank you for pointing out the monolog v3 conflict!

No worries.

I guess there'll need to be a new (major, breaking) version of the base package tagged before this package can have the dependency raised to that new version before a new major (also breaking) release can be tagged here.

michaelscola commented 1 year ago

@danielmorell @jonnott I have a new pull request for the updates for Monolog. Some of the properties that you overwrite are Readonly and ->with has unexpected behavior, so it was easier to create a new Instance with the additional data rollbar wants to add. I have tested it and everything appears to be working as expected, look forward to hearing from you with your comments.

jonnott commented 1 year ago

@danielmorell Do you know for what reason 8.0.0 isn't showing up on packagist.org yet? It's been tagged for 8 hours now.

danielmorell commented 1 year ago

@danielmorell Do you know for what reason 8.0.0 isn't showing up on packagist.org yet? It's been tagged for 8 hours now.

Hmm. Let me see if I can fix that.

jonnott commented 1 year ago

Hmm. Let me see if I can fix that.

You fixed it! 👍