Skip to content

Conversation

@stayallive
Copy link
Collaborator

@stayallive stayallive commented Nov 9, 2022

Currently we ask users to implement this in their error handler:

public function register()
{
    $this->reportable(function (Throwable $e) {
        if (app()->bound('sentry')) {
            app('sentry')->captureException($e);
        }
    });
}

However going forward we are going to change this to:

public function register()
{
    $this->reportable(function (Throwable $e) {
        \Sentry\Laravel\Integration::captureUnhandledException($e);
    });
}

The old way will still work but the "new way" will make sure exception are correctly marked as unhandled in the Sentry UI.

Fixes #607 (TODO: Docs PR)

@stayallive stayallive marked this pull request as ready for review November 9, 2022 10:29
Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

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

Should we also update the Monolog handler?

$this->hub->captureException($exception);

@stayallive
Copy link
Collaborator Author

I did thought about that, however, it's not certain that we have a unhandled exception there since a user could also just log an exception like they would use captureException, so it wouldn't be a 100% correct. WDYT?

@cleptric
Copy link
Member

Fair point, so let's leave it as is.

@stayallive stayallive merged commit f919570 into master Nov 10, 2022
@stayallive stayallive deleted the capture-unhandled-exception-proxy branch November 10, 2022 11:42
@cleptric cleptric added this to the 3.1.0 milestone Nov 10, 2022
@devfrey
Copy link

devfrey commented Nov 14, 2022

After switching from captureException() to captureUnhandledException() I'm no longer seeing any additional context in Sentry. No headers, no user, no breadcrumbs.

I'm using v3.1.0 of sentry/sentry-laravel.

@stayallive
Copy link
Collaborator Author

@devfrey you are right... fixing that now @ #611!

@stayallive
Copy link
Collaborator Author

stayallive commented Nov 14, 2022

3.1.1 was just released fixing this, sorry for that!

@devfrey
Copy link

devfrey commented Nov 14, 2022

Thanks! Can confirm the patch fixed it.

All exceptions are now reported as unhandled. Even the ones manually logged through Laravel's report() helper – which I use to 'soft'-report exceptions that didn't crash the application. How did you intend to make a distinction between handled and unhandled exceptions? Laravel doesn't have a concept of handled vs. unhandled exceptions.

@cleptric
Copy link
Member

@devfrey @axlon 3.1.2 was just released that properly sets the handled property when using report().

@cleptric
Copy link
Member

Yes, report_if() and report_unless() are also set to handled: true 🙂

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.

Correctly mark unhandled exceptions as handled: false

4 participants