getsentry / team-mobile

Meta issues for the Mobile team
4 stars 1 forks source link

HTTP transport should receive envelope when no disk space in device #52

Open bruno-garcia opened 2 years ago

bruno-garcia commented 2 years ago

In SDKs we store events on disk before attempting to send to the network, it's possible we fail to write to disk due to full disk space. In such cases, we should be OK with not persisting the envelope to disk, and proceed with sending the data to the network.

When the device is offline, the event will be lost. But there's no reason to throw it away if we can't write to disk but the device is actually online.

marandaneto commented 1 year ago

Flutter and RN call captureEnvelope for iOS, so this has to be implemented on iOS. On Android, we actually write the envelope to the disk first sync., what we could here is if the writing fails, we call captureEnvelope directly, the event isn't going to be enriched with device context, but it's better than totally lost.

denrase commented 3 months ago

@bruno-garcia Was this implemented on any of the hybrid SDKs?

romtsn commented 3 months ago

@denrase I think in java it's kinda working already, because we swallow any exception when writing the envelope to disk: https://github.com/getsentry/sentry-java/blob/d4b1f82d2bdf3415427ba550509483bbf210d787/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java#L306-L312

so it'll just proceed in the AsyncHttpTransport and try to send it I believe, but has to be tested.

denrase commented 3 months ago

@romtsn Looked through he code & tests. Since we test against IEnvelopeCache interface, and the store method does not throw, we can't really test this case in AsyncHttpTransport.

There main flow is storing the envelope, sending and then deleting the envelope from cache. This order is indeoenden from the cache failing internally.

So I'd say this is done on Android? Wdyt?

denrase commented 3 months ago

Also on iOS/Android plugins we did move to calling captureEnvelope on the native SDKs.