Skip to content

Add handled/unhandled exception support #674

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
Feb 3, 2023

Conversation

cs278
Copy link
Contributor

@cs278 cs278 commented Nov 16, 2022

@cs278 cs278 marked this pull request as ready for review November 16, 2022 16:59
@cs278 cs278 force-pushed the mark-exceptions-as-unhandled branch 2 times, most recently from defc885 to 45a7c8c Compare November 17, 2022 09:37
@cs278
Copy link
Contributor Author

cs278 commented Nov 17, 2022

Need a bit of direction, right now this supports pre 3.11.0 versions of the SDK, I can either:

  • Keep that and squash the commits
  • Remove the commit and change the Composer requirement

@cleptric
Copy link
Member

Thanks for the contribution. I can release a new version of sentry/sdk, so we can pin it to 3.4. No need for BC checks.

@cleptric
Copy link
Member

Cool stuff!
I wonder if we should add a small helper method that encapsulates the boilerplate for

$hint = EventHint::fromArray([
    'exception' => $event->getError(),
    'mechanism' => new ExceptionMechanism(ExceptionMechanism::TYPE_GENERIC, false),
]);

$this->hub->captureEvent(Event::createEvent(), $hint);

@cs278
Copy link
Contributor Author

cs278 commented Nov 25, 2022

I wonder if we should add a small helper method that encapsulates the boilerplate for

I looked at this but didn't see anywhere obvious to put such a helper, I figured ideally the HubInterface would have a captureUncaughtException() method.

@cleptric
Copy link
Member

Due to our Unified API, we can't add functions to the Hub that easily.
Pslam and PHPStan are complaining, we can merge once this is fixed.

@cs278 cs278 force-pushed the mark-exceptions-as-unhandled branch from a458d86 to 2530929 Compare November 25, 2022 14:39
@cs278
Copy link
Contributor Author

cs278 commented Nov 25, 2022

Update hopefully it passes - all fine locally apart from Psalm which seems to be emitting nonsense errors. 😖

@cs278 cs278 force-pushed the mark-exceptions-as-unhandled branch from 2530929 to b2530c9 Compare November 25, 2022 15:15
@cleptric cleptric changed the base branch from master to develop November 28, 2022 11:49
@cs278 cs278 force-pushed the mark-exceptions-as-unhandled branch from b2530c9 to 1acc6a8 Compare November 29, 2022 16:52
@cs278
Copy link
Contributor Author

cs278 commented Nov 29, 2022

@cleptric I've updated this again.

@cs278
Copy link
Contributor Author

cs278 commented Dec 5, 2022

@cleptric not sure how I can fix the failing code coverage thing, seems to be missing coverage on parenthesis?

@cleptric
Copy link
Member

cleptric commented Dec 5, 2022

@cs278 No worries, this is fine.

I'll need to test the message stuff, but I'm currently busy with other things.
So please bare with me if this will stay open for a little while.

@cs278
Copy link
Contributor Author

cs278 commented Jan 18, 2023

@cleptric polite request for an update. I'm ready to ship changes ditching our custom Sentry 1 Symfony integration and this is the only roadblock left in the way.

@cleptric
Copy link
Member

I'm still focusing on other SDKs at the moment. Can't really give you a timeline, sorry 😕

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.

Thanks again for your work on this and sorry for the delay 🙂
LGTM! 🚢

@cleptric cleptric merged commit 962a8cf into getsentry:develop Feb 3, 2023
@cleptric cleptric added this to the 4.6.0 milestone Feb 3, 2023
@cs278 cs278 deleted the mark-exceptions-as-unhandled branch February 3, 2023 12:34
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
2 participants