orta / ARAnalytics

Simplify your iOS/Mac analytics
MIT License
1.83k stars 217 forks source link

filtering out nil view controller titles: 'ARAnalyticsShouldFire' block not being called for UIViewController subclasses #171

Closed cbowns closed 9 years ago

cbowns commented 9 years ago

I'm using the aspect-oriented DSL to setup my analytics events, and had included a snippet exactly like the one in https://github.com/orta/ARAnalytics#aspect-oriented-dsl to opt any of my UIViewController subclasses which lacked a title property out of my title-oriented tracking.

However, I'm getting runtime crashes on title being nil, and in ARDSL.m's ar_shouldFireForInstance (this code), the dictionary being passed in lacks a dictionary[ARAnalyticsShouldFire] value:

(lldb) bt
* Thread 1 (tid 0xd06659, queue "com.apple.main-thread"): 0x000000010a228cb5 ar_shouldFireForInstance(dictionary=0x00007fb688f70ac0, instance=0x00007fb68b005950, context=0x00007fb688d871b0) + 133 at ARDSL.m:22
    Stopped by breakpoint 3.1
  * #0  0x000000010a228cb5 ar_shouldFireForInstance(dictionary=0x00007fb688f70ac0, instance=0x00007fb68b005950, context=0x00007fb688d871b0) + 133 at ARDSL.m:22
    #1  0x000000010a229baa __53+[ARAnalytics(.block_descriptor=0x00007fb68b0066e0, parameters=0x00007fb688d871b0) addScreenMonitoringAnalyticsHook:]_block_invoke_4 + 106 at ARDSL.m:109
    #2  0x000000010a3edaef -[RACSubscriber sendNext:](self=0x00007fb68b006510, _cmd=0x000000010a48c6e7, value=0x00007fb688d871b0) + 239 at RACSubscriber.m:72
    #3  0x000000010a3ae1ec -[RACPassthroughSubscriber sendNext:](self=0x00007fb68b006770, _cmd=0x000000010a48c6e7, value=0x00007fb688d871b0) + 444 at RACPassthroughSubscriber.m:74
    #4  0x000000010a3ecd34 __23-[RACSubject sendNext:]_block_invoke(.block_descriptor=0x00007fff55a5ba00, subscriber=0x00007fb68b006770) + 68 at RACSubject.m:93
    #5  0x000000010a3ecb33 -[RACSubject enumerateSubscribersUsingBlock:](self=0x00007fb68b0061a0, _cmd=0x000000010a4a3184, block=0x00007fff55a5ba00) + 595 at RACSubject.m:85
    #6  0x000000010a3eccc7 -[RACSubject sendNext:](self=0x00007fb68b0061a0, _cmd=0x000000010a48c6e7, value=0x00007fb688d871b0) + 151 at RACSubject.m:92
    #7  0x000000010a39618b RACForwardInvocation(self=0x00007fb68b005950, invocation=0x00007fb688d99330) + 347 at NSObject+RACSelectorSignal.m:55
    #8  0x000000010a395f6c __RACSwizzleForwardInvocation_block_invoke(.block_descriptor=0x00007fb68b005f30, self=0x00007fb68b005950, invocation=0x00007fb688d99330) + 92 at NSObject+RACSelectorSignal.m:79
    #9  0x000000010d799f4f ___forwarding___ + 495
    #10  0x000000010d799cd8 __forwarding_prep_0___ + 120
    #11  0x000000010c30eff1 -[UIViewController _setViewAppearState:isAnimating:] + 567
    #12  0x000000010d786026 __53-[__NSArrayI enumerateObjectsWithOptions:usingBlock:]_block_invoke + 70
    #13  0x000000010d785f5c -[__NSArrayI enumerateObjectsWithOptions:usingBlock:] + 284
    #14  0x000000010c30f149 -[UIViewController _setViewAppearState:isAnimating:] + 911
    #15  0x000000010c336fb9 -[UINavigationController viewDidAppear:] + 173
    #16  0x000000010c30eff1 -[UIViewController _setViewAppearState:isAnimating:] + 567
    #17  0x000000010c349f26 -[UITabBarController viewDidAppear:] + 108
    #18  0x000000010d732dec __invoking___ + 140
    #19  0x000000010d732c42 -[NSInvocation invoke] + 290
    #20  0x000000010a39612c RACForwardInvocation(self=0x00007fb688c4dc10, invocation=0x00007fb688c9cf20) + 252 at NSObject+RACSelectorSignal.m:50
    #21  0x000000010a395f6c __RACSwizzleForwardInvocation_block_invoke(.block_descriptor=0x00007fb688f8b350, self=0x00007fb688c4dc10, invocation=0x00007fb688c9cf20) + 92 at NSObject+RACSelectorSignal.m:79
    #22  0x000000010d799f4f ___forwarding___ + 495
    #23  0x000000010d799cd8 __forwarding_prep_0___ + 120
    #24  0x000000010c30eff1 -[UIViewController _setViewAppearState:isAnimating:] + 567
    #25  0x000000010c30fb3b -[UIViewController _executeAfterAppearanceBlock] + 52
    #26  0x000000010c1fe62c _applyBlockToCFArrayCopiedToStack + 314
    #27  0x000000010c1fe4c5 _afterCACommitHandler + 564
    #28  0x000000010d76fca7 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    #29  0x000000010d76fc00 __CFRunLoopDoObservers + 368
    #30  0x000000010d765a33 __CFRunLoopRun + 1123
    #31  0x000000010d765366 CFRunLoopRunSpecific + 470
    #32  0x000000010f920a3e GSEventRunModal + 161
    #33  0x000000010c1da900 UIApplicationMain + 1282
    #34  0x000000010a1d102f main(argc=1, argv=0x00007fff55a5d358) + 111 at main.m:14
    #35  0x000000010e464145 start + 1
    #36  0x000000010e464145 start + 1
(lldb) po dictionary
{
    keypath = title;
}

(lldb) po instance
<BoldPostTableViewController: 0x7fb68b005950>

(lldb) po [instance title]
 nil

I was under the impression (perhaps wrongly) that this configuration would apply to all subclasses of UIViewController. (This is taken more or less verbatim from the readme for ARAnalytics.)

configuration: @{
    ARAnalyticsTrackedScreens: @[ @{
        ARAnalyticsClass: UIViewController.class,
        ARAnalyticsDetails: @[ @{
            ARAnalyticsPageNameKeyPath: @"title",
        }],
        ARAnalyticsShouldFire:
        ^BOOL(UIViewController *controller, NSArray *parameters) {
            if ( nil == controller.title ) {
                NSLog(@"%s controller %@ has no title!", __func__, controller);
            }
            return controller.title != nil;
        },
…
ashfurrow commented 9 years ago

Hmm. Sorry about that @cbowns.

Taking a look, it appears that the ARAnalyticsShouldFire key/value pair should be inside the ARAnalyticsDetails. This lets you have per-event firing filtering behaviour. In the code you've posted above, ARAnalytics doesn't see the should fire block and assumes there isn't one.

Please let us know if this helps.

(Side note: this is actually a weakness of the DSL – you are not the first person to have this issue. In fact, I made the mistake myself a few weeks ago. @kylef has suggested moving away from arrays/dictionaries. Maybe we can use Swift? Open to suggestions.)

cbowns commented 9 years ago

Oh, that makes sense! I simply didn't read the DSL examples that closely, and Xcode's default formatting made it difficult to see the scope of what I'd added. That fixed it, thanks.

ashfurrow commented 9 years ago

:+1: