Skip to content

Introduce captureUnhandledException #608

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 2 commits into from
Nov 10, 2022

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
3 participants