sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

more defensive on eventid header response

Open SimonCropp opened this issue 3 years ago • 2 comments

related to #1657

#skip-changelog

for the event_id header:

  • if missing return null (i am surprised this is valid?)
  • if null log error (throw in debug) and return null
  • if not a string log error (throw in debug) and return null. IMO this should always throw a exception
  • if fails to parse as a guid log error (throw in debug) and return null. IMO this should always throw a exception
  • if guid is empty log error (throw in debug) and return SentryId.Empty. IMO this should always throw a exception
  • else return new SentryId with the guid

SimonCropp avatar Oct 03 '22 01:10 SimonCropp

RE:

if missing return null (i am surprised this is valid?)

Not all envelopes contain "events". In particular, a session update will not have an event_id of its own. If a session is sent by itself, such as at the start of a session, then TryGetEventId should return null

mattjohnsonpint avatar Oct 06 '22 18:10 mattjohnsonpint

We could improve debug messages for such cases. For example, the start of a session looks like this in the debug output:

  Debug: Envelope '' sent successfully. Payload: {"sdk":{"name":"sentry.dotnet","version":"3.21.0"},"sent_at":"2022-10-06T18:11:36.785535+00:00"} {"type":"session","length":293} {"sid":"6a09b56686834e67b5d8042c60ec2799","did":"4e01e65c-8951-4610-ac6b-7805e72269d9","init":true,"started":"2022-10-06T11:11:36.548873-07:00","timestamp":"2022-10-06T18:11:36.549708+00:00","seq":0,"duration":0,"errors":0,"attrs":{"release":"2022-10-06T11:11:36","environment":"development"}}

The message Envelope '' sent successfully. looks like a bug, but it's just that the envelope contains no event id. It would be better to say something like Envelope for session '6a09b56686834e67b5d8042c60ec2799' sent successfully.

Also the wording Payload: is wrong, because it's not just the payload of the envelope, but the entire serialized envelope including header and payload.

mattjohnsonpint avatar Oct 06 '22 18:10 mattjohnsonpint