Don't resend envelopes that get rejected by the server #3938
Conversation
jamescrosswell
changed the title
| catch (Exception ex) when (IsRejectedByServer(ex)) | ||
| { | ||
| _options.ClientReportRecorder.RecordDiscardedEvents(DiscardReason.SendError, envelope); | ||
| _options.LogError(ex, "Failed to send cached envelope: {0}. The envelope is likely too big and will be discarded.", file); |
There was a problem hiding this comment.
Don't we have a specific "too big" client report? It's possible we'll get that from the server though in which case this is fine.
But when debugging data loss on behalf of customers, when we see "send error" it's often hard to know what to do next.
There was a problem hiding this comment.
Just heard that Relay doesn't report anything btw. So we must
There was a problem hiding this comment.
We don't no. According to the docs:
The following discard reasons are currently defined for discarded_events:
- queue_overflow: a SDK internal queue (eg: transport queue) overflowed
- cache_overflow: an SDK internal cache (eg: offline event cache) overflowed
- buffer_overflow: an SDK internal buffer (eg. breadcrumbs buffer) overflowed
- ratelimit_backoff: the SDK dropped events because an earlier rate limit instructed the SDK to back off.
- network_error: events were dropped because of network errors and were not retried.
- sample_rate: an event was dropped because of the configured sample rate.
- before_send: an event was dropped in before_send
- event_processor: an event was dropped by an event processor; may also be used for ignored exceptions / errors
- send_error: an event was dropped because of an error when sending it (eg: 400 response)
- internal_sdk_error: an event was dropped due to an internal SDK error (eg: web worker crash)
- insufficient_data: an event was dropped due to a lack of data in the event (eg: not enough samples in a profile)
- backpressure: an event was dropped due to downsampling caused by the system being under load
I couldn't see anything that matched up neatly with the situation we're experiencing so I used send_error. Possibly buffer_overflow is better here though?
There was a problem hiding this comment.
Since there wasn't anything better, I went with buffer_overflow
| { | ||
| // TODO: Envelopes could end up in an infinite loop here. We should consider implementing some |
There was a problem hiding this comment.
You could end up with a lot of items in queue causing new data to not get a chance to be pushed on mobile platforms. Can the queue be sorted to send new data first until a retry/circuit breaker can be added?
There was a problem hiding this comment.
Priority queues (e.g. prioritise error reports over traces) is one for the todo list:
Resolves #3804:
Windows
Interestingly, on Windows the connection closes gracefully and we get a nice orderly response code back from the server that we can process:
This PR may actually be working around an issue in the .NET Runtime on macOS then.