Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
41 stars 12 forks source link

Use Sentry `flush` to send events immediately #232

Closed mokagio closed 1 year ago

mokagio commented 1 year ago

This PR removes our custom logic to implement the "send all enqueue events and wait" functionality in favor of using the new Sentry's native flush method.

One big disadvantage of this approach is that we lose the precise callback management, because flush is blocking for as long as it takes to send all events or for a given timeout to run out.

I used the demo app to verify our custom implementation works and the events it sends reach Sentry. In the screenshot below, the top two events have been sent from this branch. Also notice the third and fourth events have "(no value)" for release and environment. That's a bug introduced at some point when migrating to version Sentry version 7. I haven't looked into it yet.

image

I'm of two minds on whether to go for using flush under the hood. I really dislike how flush doesn't provide a callback out of the box, and how it results in a less precise UI state handling on our end. At the same time, relying on Sentry's native code instead of implementing our own logic accessing their private API simplifies and de-risks the library.

I'd like other folks input on this, but I'm tempted to adopt this implementation on the basis that logErrorAndWait, logErrorImmediately, and logErrorsImmediately are used very sparingly by the library's clients. Namely, in one occasion in Jetpack/WordPress and in two occasions in WooCommerce](https://github.com/woocommerce/woocommerce-ios/search?q=logFatalErrorAndExit).