getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
606 stars 206 forks source link

Maui app not sending data to Sentry - KeyNotFoundException: The given key was not present in the dictionary. #3534

Closed MichaelLHerman closed 3 months ago

MichaelLHerman commented 3 months ago

Package

Sentry

.NET Flavor

Other

.NET Version

8.0.0

OS

iOS

SDK Version

4.9.0

Self-Hosted Sentry Version

No response

Steps to Reproduce

Started MAUI app with span logging using AspectInjector

Seeing exception in debug output

Image

Span does not have timestamp

{"span_id":"5795b574ac8daf85","parent_span_id":"58eed2aeb59cdcae","trace_id":"2c5aacd8af3a41f39627af775bf9bd51","op":"realm","description":"RealmService.Find(Prefs)","start_timestamp":"2024-08-14T18:17:29.104403+00:00"}

debug output.txt

Expected Result

Client side code should be robust to not stop logging completely if there is invalid data

Actual Result

1723659550_-8129dc3ca5f832e646c78ff9aa05bc795d6e-673663803.envelope.txt

MichaelLHerman commented 3 months ago

This might be from upgrading to 4.10 and then downgrading to 4.9

jamescrosswell commented 3 months ago

Hi @MichaelLHerman , thanks for reporting this. I don't think it has anything to do with 4.9 vs 4.10. It looks like there's an asymmetry between the WriteTo and FromJson methods for Span:

WriteTo

writer.WriteStringIfNotNull("timestamp", EndTimestamp);

FromJson:

var endTimestamp = json.GetProperty("timestamp").GetDateTimeOffset();

It's odd is that you're seeing this error. I think the only time the SentrySDK tries to deserialize JSON envelopes is if they've been cached to disk by the CachingTransport: https://github.com/getsentry/sentry-dotnet/blob/d5d4b663b4c6c9ba5fc23f3a82cc66a555cfdcac/src/Sentry/Internal/Http/CachingTransport.cs#L326

And in that case, if an exception is thrown then it's logged and the envelope is discarded. It's also odd that the span isn't being finished. Ordinarily this wouldn't crash the application then. Are you able to create a minimal reproduction so that we could work out why it's crashing?

Regardless, there's a logical error in the FromJson method that needs fixing (addressed in #3533)... but it'd be good to ensure if any similar errors are introduced in the future, they impact SDK users's minimally.

bitsandfoxes commented 3 months ago

Are you able to create a minimal reproduction so that we could work out why it's crashing?

The repro I ran into: Have a terminal exception happen while a span is active. The SDK runs into here https://github.com/getsentry/sentry-dotnet/blob/860edc38aeb9c34d4820d1fe8f9565c5b4d9b88a/src/Sentry/Internal/Hub.cs#L439-L445 and finishes the transaction in an attempt to (I assume) preserve "what we have so far".

MichaelLHerman commented 3 months ago

@bitsandfoxes

Its a bit hard for me to get a minimal reproduction, but I think it might be happening due to the way I'm using scope.Transaction and multiple simultaneous transactions in Sentry.Maui..

My app is offline capable, so I have a database and a "sync queue" to make service calls. I'm using Realm as my db, around which I instrument each database call by wrapping in a span. The span attempts to attach to the ambient transaction. _hub.GetSpan()?.StartChild("realm", $"get {id}")

A UI action, like submitting a form, will start a transaction, write data to the database, and enqueue a sync task to send the data.

_transaction = _hub.StartTransaction(NAME_OF_COMMAND, "ui.command");
_hub.ConfigureScope(scope => scope.Transaction = _transaction);

The sync queue is a loop looking for work. If it has an item in its queue, it will start a transaction, also setting it to the current transaction, read from the database, make an API call, and write to the database.

I'm really not sure how to do multiple simultaneous transactions in Sentry.Maui. I really don't want to have to pass transactions from ViewModel -> DataService -> Database or SyncService -> DataService -> Database

I see IsGlobalModeEnabled is hardcoded on for client-side apps. Would it help to turn it off?

        builder.Services.PostConfigure((SentryMauiOptions options) =>
        {
            //need to manually set here because it's overridden by Configure
            options.IsGlobalModeEnabled = false;
        });
bitsandfoxes commented 3 months ago

Hey @MichaelLHerman, thanks for the additional context! I don't think there's anything else we need to deal with this since the test added in #3533 let's us repro this and we know where the transaction finishing originates from. Version 4.10.1 contains the fix, could you give that a try?

MichaelLHerman commented 3 months ago

@bitsandfoxes 4.10.1 seems good.

I will open a new issue regarding multiple simultaneous transactions / IsGlobalModeEnabled