Skip to content

Also report exception HResult #3939

Closed

Description

@whiskhub

Problem Statement

Some exceptions, especially COMException or Win32Exception, sometimes do not provide a good exception message which makes it hard to analyze issues on Sentry. COMException even often returns an empty exception message.

However, in .NET every Exception object has a HResult property. Especially for the cases mentioned above, this value would be worth a lot as it contains the concrete error type (e.g. -2147417842 -> RPC_E_WRONG_THREAD). This HResult property is currently not sent with a Sentry event.

Therefore I would like to request the HResult property of an Exception to also be uploaded in a SentryEvent, and that its value is also visible in the Sentry web UI.

Solution Brainstorm

According to your documentation, such information could be stored in the Exception.Mechanism.Meta field (https://develop.sentry.dev/sdk/data-model/event-payloads/exception/#meta-information).

However, there are currently only Linux/Apple specific definitions listed, like signal or mach_exception. A definition for a Windows HResult, or more generally HResult (as they may also be used in cross-platform .NET apps), should be added there.

Using ISentryEventExceptionProcessor I already tried manually adding some HResult field to the Meta. However, it was not displayed in the Sentry web UI. Only when I used errno to "fake" a HResult, it was showing up. So I assume you must also add server-side handling.

For the .NET side, the change should be fairly easy. In the MainExceptionProcessor, something alike should be added:

mechanism.InternalMeta = new()
{
    { "HResult", exception.HResult }
};

Activity

  1. moved this to Waiting for: Product Owner in GitHub Issues with 👀 3on Feb 5, 2025
  2. jamescrosswell commented on Feb 5, 2025

    @jamescrosswell
    Collaborator

    Thanks for the suggestion @whiskhub - it's a good idea!

    In the mean time, you could easily write your own processor to add this information to events:

    internal class ComExceptionProcessor : ISentryEventExceptionProcessor
    {
        public void Process(Exception exception, SentryEvent sentryEvent)
        {
            // Add the HResult to the event's extra data.
            sentryEvent.SetExtra("HResult", exception.HResult);
        }
    }

    ... and then use this when initialising Sentry:

    SentrySdk.Init(options =>
    {
        // All your normal options
    
        // Wire up the custom event processor
        options.AddExceptionProcessor(new ComExceptionProcessor());
    });
  3. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 3on Feb 5, 2025
  4. Lightczx commented on Mar 4, 2025

    @Lightczx
    Contributor

    @jamescrosswell If the COMException is aggregated or is a inner exception, then your code will failed to handle them.
    And if there are multiple COMExceptions in the parent exception, and they may have different HResult values. I think get only the parent's hresult is not enough for WinUI 3 or WinRT.

  5. moved this to Waiting for: Product Owner in GitHub Issues with 👀 3on Mar 4, 2025
  6. jamescrosswell commented on Mar 4, 2025

    @jamescrosswell
    Collaborator

    Good call @Lightczx.

    Sentry unpacks aggregate exceptions and will show multiple exceptions (where appropriate) for a single event...

    So the HResult needs to be stored on the SentryException rather than the SentryEvent (or it needs to be stored on the event in a convoluted way that lets you know which exception it relates to)... So back to what @whiskhub suggested - the UI needs to be able to display this if it is stored in the Mechanism metadata basically.

  7. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 3on Mar 4, 2025
  8. bruno-garcia commented on Mar 4, 2025

    @bruno-garcia
    Member

    I think it makes sense to add this OOTB. Sentry could even link to the right place if that's helpful too. Or have some mappigns in the server so the UI shows additional context other than just the codes

  9. 4 remaining items

  10. moved this to Waiting for: Product Owner in GitHub Issues with 👀 3on Mar 7, 2025
  11. bruno-garcia commented on Mar 7, 2025

    @bruno-garcia
    Member

    @Lightczx would you be willing to add a PR to the repo with this feature?

    Not sure about .ZIp as the approach, but conceptually I agree that any HResult in an exception should be added to the Sentry exception, so it shows up as it did on your screenshot

  12. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 3on Mar 7, 2025
  13. Lightczx commented on Mar 8, 2025

    @Lightczx
    Contributor

    would you be willing to add a PR to the repo with this feature?

    I can make a PR to adjust the MainExceptionProcessor, then no .Zip() call needed, HResult can just set here.

    But I was also thinking, should we provide an option to disable the new behavior? @bruno-garcia

  14. moved this to Waiting for: Product Owner in GitHub Issues with 👀 3on Mar 8, 2025
  15. added a commit that references this issue on Mar 8, 2025
  16. moved this from Waiting for: Product Owner to No status in GitHub Issues with 👀 3on Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FeatureNew feature or request

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions