Fix: dispatch autoheartbeat timers, send offline event dispatch

at-internet / piano-analytics-apple

Piano Analytics Apple SDK repository

https://developers.atinternet-solutions.com/piano-analytics/

MIT License

7 stars 13 forks source link

Fix: dispatch autoheartbeat timers, send offline event dispatch #13

Closed aregler closed 1 year ago

aregler commented 1 year ago

This PR fixes

an issue where timers were not dispatched on the main queue, such that no heartbeat events were fired at all
an issue where offline events were processed on the main queue (kinda unperformant because of the semaphore semantics when sending the event)
log warnings where the FileManager tried to delete non-existing files

BenDz commented 1 year ago

Hey @aregler Thanks for your PR! We will review it asap

warchimede commented 1 year ago

Hello @BenDz,

The new 3.0.5 release from last week is conflicting with changes made in this PR.

Does it mean that the most recent version includes the fixes provided by @aregler or some similar code ? If it is not the case, do you know when this PR will be reviewed and included ?

We are really looking forward to integrating a new version of Piano Analytics in which these issues are dealt with.

Thanks in advance 🙏🏽

aregler commented 1 year ago

Hey. Currently I am on parental leave and will review/update my PR(s) in a few weeks.

aregler commented 1 year ago

Hello @BenDz,

I updated my PR. The timers still were not dispatched on the main queue. That's the reason why in our project no autoheartbeat timers would be fired at all.

Additionally I refactored the sendStoredChunk method. Previously the offline event was sent and afterwards the offline file was deleted. This could lead to duplicate events, because the offline events are sent with a delay and the processSendEvents method is called multiple times. Having a greater number of offline evens (n > 10 was sufficient for my tests) could be in queue here:

while the send method is called again. The offline files were not deleted at this point and simply enqueued again.

BenDz commented 1 year ago

Hello, Thank you for your inputs. We are currently reviewing it and plan to release it beginning of September along with our next update.