Skip to content
This repository has been archived by the owner on Aug 18, 2023. It is now read-only.

fix(analytics-process): fix retrieving events from storage #9

Merged
merged 5 commits into from Dec 9, 2020

Conversation

dlenam
Copy link
Contributor

@dlenam dlenam commented Dec 7, 2020

Fixes the case where the storage is overwritten by the first sent events.

The buggy flow:

  1. Init the MixpanelAnalytics class, that schedules the periodic process (Timer.periodic(...)) that pushes the events, prior checking if it's the first cycle and pulling events from memory.
  2. Push new events that are stored in the shared prefs right away.
  3. Close the app before the events are sent.
  4. Reopen the app, and init MixpanelAnalytics that starts the Timer.periodic.
  5. Push events before the first cycle of the Timer.periodic happens. At this point no interaction with the sharedprefs happened, and the events stored in the queues of the driver are empty. Although there are old events stored in memory. The new events gets added to the queue and also stored in memory, overwriting the old ones.
  6. First time the Timer.periodic process executes, memory is checked, but we only have the new events as the old ones were lost.

An option is to add some init() method that would retrieve the old events from sharedprefs as soon as the driver starts. But as that is an async process that cannot happen in a constructor (could be some sort of static async factory method) it would be a breaking change. So this is an intermediate solution, where we no longer check for any previous stored events in the Timer.periodic process but in the actual methods where the events are pushed, thus avoiding the overwriting. This solution has the downside that we would only send old events in the scenario where new events are pushed. If no new events are pushed, then any old event would be ever sent.

This also updates the lock dependencies to flutter stable 1.22

Fixes the case where the storage is overwritten by the first sent events. Better to handle this in
some init() method, but that would be a breaking change.
@dlenam dlenam requested a review from a team December 7, 2020 16:12
Copy link
Contributor

@ogasquez ogasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dlenam dlenam merged commit bd588c7 into master Dec 9, 2020
@robertohuertasm robertohuertasm deleted the fix/from-storage-events-case branch February 11, 2021 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants