Closing Unfinished Transactions

[mattjohnsonpint]

Consider:

public void Main()
{
    SentrySdk.Init(...);
    var transaction = SentrySdk.StartTransaction("foo", "bar");
    DoSomething();
    transaction.Finish();
}

If DoSomething throws an unhandled exception, the transaction is never finished and thus not sent to Sentry. We should consider whether we should catch this in and finish the transactions with an error status, so that information is not lost.

We could just do that in our unhandled exception handlers, or when capturing an exception that is marked unhandled (so it works with all integrations). However, that doesn't take care of this possibility:

public void Main()
{
    SentrySdk.Init(...);

    try
    {
        var transaction = SentrySdk.StartTransaction("foo", "bar");
        DoSomething();
        transaction.Finish();
    }
    catch (Exception e)
    {
        SentrySdk.CaptureException(e);
    }
}

Of course, that should probably be written as:

public void Main()
{
    SentrySdk.Init(...);

    try
    {
        var transaction = SentrySdk.StartTransaction("foo", "bar");
        DoSomething();
        transaction.Finish();
    }
    catch (Exception e)
    {
        transaction.Finish(SpanStatus.UnknownError);
        SentrySdk.CaptureException(e);
    }
}

But we could also do that if we made it IDisposable, such as:

public void Main()
{
    SentrySdk.Init(...);

    try
    {
        using var transaction = SentrySdk.StartTransaction("foo", "bar");
        DoSomething();
        transaction.Finish();
    }
    catch (Exception e)
    {
        SentrySdk.CaptureException(e);
    }
}

... which then simplifies the original problem to:

public void Main()
{
    SentrySdk.Init(...);

    using var transaction = SentrySdk.StartTransaction("foo", "bar");
    DoSomething();
    transaction.Finish();
}

Unfortunately, that would be a breaking change - and one that is easily missed.

An alternative API that we could add without breaking anything would be:

public void Main()
{
    SentrySdk.Init(...);

    SentrySdk.WithTransaction("foo", "bar", transaction => {
        DoSomething();
        transaction.Finish();
    });
}

[mattjohnsonpint]

Some previous discussion on this here: getsentry/develop#443

Also relates to #1844

[mattjohnsonpint]

My inclination is to do two things:

1. In Hub.CaptureEvent we already have some logic for marking sessions as crashed for unhandled exceptions. We could also finish open transactions there with an error status.
2. Add the SentrySdk.WithTransaction API described above to provide a better pit of success. Mark StartTransaction obsolete in favor of WithTransaction.

We may also need to consider if we need to finish unfinished spans in the same way or not.

[mattjohnsonpint]

For now, let's forgo the API change and just focus on closing the transaction with an error status (See 1 above).

[mattjohnsonpint]

Probably should return Aborted as a status?

sentry-dotnet/src/Sentry/SpanStatus.cs

Line 56 in 19da8b8

Aborted,