Skip to content
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

ref: Use the global LogHandler to capture unhandled Exceptions #731

Merged
merged 29 commits into from
May 31, 2022

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented May 6, 2022

Turns out the Unity Runtime actually "handles" uncaught Exceptions from scripts and hands them off to its global LogHandler. This is presumably the reason why AppDomain.UnhandledException is never being invoked for them.

We can capture these Exceptions (and in the future enhance them with native instruction addrs) by shimming the global LogHandler.

This completely replaces and removes the Application.OnLogMessageReceived integration and the need to parse exceptions from string.

Resolves #579
Resolves #599

Swatinem and others added 7 commits May 6, 2022 15:55
Turns out the Unity Runtime actually "handles" uncaught Exceptions from scripts and hands them of to its global LogHandler. This is presumably the reason why AppDomain.UnhandledException is never being invoked for them.
We can capture these Exceptions (and in the future enhance them with native instruction addrs) by shimming the global LogHandler.
@Swatinem Swatinem changed the title WIP: ref: Use the global LogHandler to capture unhandled Exceptions ref: Use the global LogHandler to capture unhandled Exceptions May 18, 2022
@Swatinem Swatinem marked this pull request as ready for review May 18, 2022 09:22
_unityLogHandler.LogException(exception, context);
}

internal void CaptureException(Exception exception, UnityEngine.Object? context)
Copy link
Member

Choose a reason for hiding this comment

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

what's in context? Anything useful to add to the event or crumb?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the crazy awesome part: When you're calling Debug.Log("My log message", this); you can actually pass "whatever" in there as context (the MonoBehaviour in this case). Inside the editor this lets you click the log in the console and highlights the GameObject in the hierarchy. What that means for us is that we could grab all kinds of useful stuff from there.

  • The name of the GameObject
  • Other components on that GameObject
  • Child Count
  • e.t.c.

I'm just not sure what to grab. Probably the name.

src/Sentry.Unity/UnityEventProcessor.cs Show resolved Hide resolved
Copy link
Collaborator

@vaind vaind left a comment

Choose a reason for hiding this comment

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

LGTM with some questions. Also maybe add links to issues on sentry.io for before and after the change? If there's any change in the context

test/Directory.Build.props Outdated Show resolved Hide resolved
test/Sentry.Unity.Tests/IntegrationTests.cs Outdated Show resolved Hide resolved
@mattjohnsonpint
Copy link
Contributor

Looks mostly good, except the changes I suggested. But also, why is the Build - 2020 task failing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants