Fixed a bug, where the tracker would stop automatic dispatching. #169
Conversation
| @@ -96,6 +96,7 @@ final public class PiwikTracker: NSObject { | |||
| guard events.count > 0 else { | |||
| // there are no more events queued, finish dispatching | |||
| self.isDispatching = false | |||
| self.startDispatchTimer() | |||
There was a problem hiding this comment.
startDispatchTimer is called from different levels/abstractions in the code.
Eventually it might become hard to make sure all code branches are covered since the call is spread out in the code.
One option would be if dispatchBatch() took a closure that is called when it is done. Then the logic can be confined to the dispatch() method (I think).
There was a problem hiding this comment.
Maybe it is possible to cleanup the whole dispatching process. It looks a bit overly complex for what it does. I will try to create a proposal.
| @@ -96,6 +96,7 @@ final public class PiwikTracker: NSObject { | |||
| guard events.count > 0 else { | |||
| // there are no more events queued, finish dispatching | |||
| self.isDispatching = false | |||
There was a problem hiding this comment.
Just noticed that isDispatching is not thread safe, depending on what thread the caller calls the dispatch() method. There is a risk that the timer is not started/stopped as it should.
There was a problem hiding this comment.
I would fix that in a separate PR since the current bug is critical
PiwikTracker/PiwikTracker.swift
Outdated
| @@ -105,15 +106,20 @@ final public class PiwikTracker: NSObject { | |||
| self.dispatchBatch() | |||
| }) | |||
| }, failure: { shouldContinue in | |||
| self.logger.warning("Failed dispatching events with error \(shouldContinue)") | |||
There was a problem hiding this comment.
Not part of this PR but what is shouldContinue? From the name I would say it's a Bool, but it is logged in the warning as error?
There was a problem hiding this comment.
Thanks for that. I just renamed it.
brototyp
force-pushed
the
bugfix/167-fixed-stalling-dispatching
branch
from
37890b3 to
332bbe0
Compare
No description provided.