Countly / countly-sdk-ios

Countly Product Analytics iOS SDK with macOS, watchOS and tvOS support.
https://count.ly
MIT License
617 stars 244 forks source link

Crash on startup #73

Closed ddaddy closed 8 years ago

ddaddy commented 8 years ago

I've just added the latest SDK update to my app, and on startup it crashes on line 97 of CountlyPersistency.m

*** Terminating app due to uncaught exception 'NSGenericException', reason: '*** Collection <__NSArrayM: 0x7f8ad0e02b50> was mutated while being enumerated.'

erkanyildiz commented 8 years ago

If you have used a beta version of the SDK before, please make sure you delete the app and reinstall again to clean persistency file.

ddaddy commented 8 years ago

This was in the Simulator, and I tried resetting the sim and it still crashes.

erkanyildiz commented 8 years ago

1- Just to confirm, you have used beta versions of the SDK before? 2- Sometimes resetting simulator does not clean all old files. Can you change simulator version? 3- What happens when you run on real device? 4- Can you share the code how you start Countly? 5- And after setting COUNTLY_DEBUG flag, can you share console logs?

ddaddy commented 8 years ago

1- Beta versions of which SDK? I have used Countly for years with the old country-ios-sdk 2- I have tried 3 different simulators. 3- Also crashes on device 4- In AppDelegate didFinishLaunchingWithOptions I use the standard

[[Countly sharedInstance] startWithMessagingUsing:countlyId
                                             withHost:@"http://ec2-xx-xx-xx-xx"
                                           andOptions:launchOptions];

5-

2016-02-20 10:37:24.666 MyApp[13235:276697] [Crashlytics] Version 3.4.1 (92) 2016-02-20 10:37:24.679 MyApp[13235:276697] Retrieved Device ID: (null) 2016-02-20 10:37:24.685 MyApp[13235:276697] Request started http:///i?app_key=&device_id=(null)&timestamp=1455903192&hour=17&dow=5&sdk_version=16.02&session_duration=0 2016-02-20 10:37:24.911 MyApp[13235:276697] Sending APN token in mode 1 2016-02-20 10:37:24.911 MyApp[13235:276697] Sending APN token in mode 1 2016-02-20 10:37:25.216 MyApp[13235:276697] [FYB Warn]: No cache network policy to apply 2016-02-20 10:37:25.400 MyApp[13235:277238] Request successfully completed 2016-02-20 10:37:25.400 MyApp[13235:276697] Request started http:///i?app_key=&device_id=(null)&timestamp=1455903192&hour=17&dow=5&sdk_version=16.02&session_duration=0 2016-02-20 10:37:25.400 MyApp[13235:277238] Request started http:///i?app_key=&device_id=(null)&timestamp=1455903435&hour=17&dow=5&sdk_version=16.02&begin_session=1&metrics=%7B%22_device%22%3A%22x86_64%22%2C%22_os_version%22%3A%229.2%22%2C%22_locale%22%3A%22en_US%22%2C%22_has_watch%22%3A0%2C%22_app_version%22%3A%221.6.1%22%2C%22_installed_watch_app%22%3A0%2C%22_resolution%22%3A%22750x1334%22%2C%22_os%22%3A%22iOS%22%7D 2016-02-20 10:37:25.402 MyApp[13235:277174] * Terminating app due to uncaught exception 'NSGenericException', reason: '* Collection <__NSArrayM: 0x7fb039e34ef0> was mutated while being enumerated.'

erkanyildiz commented 8 years ago

OK. Now I see.

You should read: http://resources.count.ly/docs/countly-sdk-for-ios-and-os-x

With v16.02 starting method of Countly changed like this:

    CountlyConfig* config = CountlyConfig.new;
    config.appKey = @"YOUR_APP_KEY";
    config.host = @"https://YOUR_COUNTLY_SERVER";
    config.features = @[CLYMessaging];
    config.launchOptions = launchOptions;  //launchOptions is required for CLYMessaging feature
    [Countly.sharedInstance startWithConfig:config];

And I will add this warning to v16.02 release notes too.

ddaddy commented 8 years ago

Thanks, that stops it crashing. I'm a bit confused over the migration of users. The documentation doesn't explain it very well.

As my current users are using OpenUDID, I need to use config.deviceID = CLYOpenUDID; With this, do I need to keep the old Countly_OpenUDID.h/.m files?

The docs also recommend migrating users using setNewDeviceID:onServer but it doesn't actually say how to use this method. How should I migrate current users over to IDFA?

ddaddy commented 8 years ago

Additionally, although not related to this. I can't find anywhere in the documentation that says what APM and ViewTracking is, so have no idea whether I need to enable it or not.

erkanyildiz commented 8 years ago

Glad you do not have crash anymore.

As my current users are using OpenUDID, I need to use config.deviceID = CLYOpenUDID;

Yes. But, if you started Countly without specifying config.deviceID as CLYOpenUDID before, SDK already initialized device ID as CLYIDFA by default. After this point, even if you set config.deviceID to CLYOpenUDID, SDK will still use CLYIDFA as it is already initialized before. To force deviceID initialization again and make sure SDK uses CLYOpenUDID, please set forceDeviceIDInitialization flag on config object.

config.forceDeviceIDInitialization = YES;

With this, do I need to keep the old Countly_OpenUDID.h/.m files?

Yes

setNewDeviceID:onServer but it doesn't actually say how to use this method.

https://github.com/Countly/countly-sdk-ios/blob/master/Countly.h#L29

Additionally, to see inner workings of this method, you can check 3. case on Custom Device ID / Changing Device ID section of SDK Development Guide http://resources.count.ly/docs/sdk-development-guide#section-custom-device-id-changing-device-id

How should I migrate current users over to IDFA?

[Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES];

erkanyildiz commented 8 years ago

APM documentation is will be added soon. It simply lets you measure your apps network performance automatically.

Automatic ViewTracking documentation will be added soon too. It simply lets you track how much time is spent on which views of your application automatically.

ddaddy commented 8 years ago

Ok, so apart from me on my test sims, none of my users have had an app update with this latest SDK, so I should use config.deviceID = CLYOpenUDID; in my next update so all my users don't appear as new users.

I should also add [Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES]; to migrate them over to IDFA for future.

So my startup config will look like

    CountlyConfig* config = CountlyConfig.new;
    config.appKey = countlyId;
    config.host = @"http://<REDACTED>.com";
    config.features = @[CLYMessaging];
    config.launchOptions = launchOptions;  //launchOptions is required for CLYMessaging feature
    config.deviceID = CLYOpenUDID;
    [Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES];
    [Countly.sharedInstance startWithConfig:config];

And it would be fine to leave it like this for a few months, then remove these 2 lines and it'll continue to use IDFA and I can remove the OpenUDID stuff?

    config.deviceID = CLYOpenUDID;
    [Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES];
ddaddy commented 8 years ago

In fact, I just realised, setNewDeviceID wants an actual ID. This update is so confusing!

Am I better just continuing with OpenUDID and forget migrating users to IDFA?

gorkem-cetin commented 8 years ago

As a side note, upon your confusion, it seems we need to come up with proper documentation for those who are willing to switch to another device ID identification method. Thanks for letting us know.

erkanyildiz commented 8 years ago

Your start up config looks great except setNewDeviceID part:

It should be like this

    CountlyConfig* config = CountlyConfig.new;
    config.appKey = countlyId;
    config.host = @"http://<REDACTED>.com";
    config.features = @[CLYMessaging];
    config.launchOptions = launchOptions;  //launchOptions is required for CLYMessaging feature
    config.deviceID = CLYOpenUDID;
    [Countly.sharedInstance startWithConfig:config];

and after starting you can call setNewDeviceID, not before starting:

[Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES];

Actually I am not sure what you mean by

In fact, I just realised, setNewDeviceID wants an actual ID.

Can you explain?

Am I better just continuing with OpenUDID and forget migrating users to IDFA?

No. I can see that you do not want to use OpenUDID anymore, even not source files. What we are doing here is same to you. We do not want to use OpenUDID anymore too.

So, as you said, after a few months all your existing users switch to IDFA you can send another update with following starting method:

    CountlyConfig* config = CountlyConfig.new;
    config.appKey = countlyId;
    config.host = @"http://<REDACTED>.com";
    config.features = @[CLYMessaging];
    config.launchOptions = launchOptions;  //launchOptions is required for CLYMessaging feature
    config.deviceID = CLYIDFA;
    [Countly.sharedInstance startWithConfig:config];

And then you can completely delete and forget OpenUDID forever. No need to set config.deviceID as CLYOpenUDID or call setNewDeviceID anymore.

ddaddy commented 8 years ago

My confusion with the setNewDeviceID method was from thinking the method wanted an actual ID, and CLYIDFA is the actual string CLYIDFA, so I wasn't sure if all my users would suddenly have the ID CLYIDFA and all be the same user. Digging deeper into the code, I see that you compare for the CLYIDFA string and generate an IDFA ID.

So now my full startup config looks like this:

    CountlyConfig* config = CountlyConfig.new;
    config.appKey = countlyId;
    config.host = @"http://<REDACTED>.com";
    config.features = @[CLYMessaging];
    config.launchOptions = launchOptions;  //launchOptions is required for CLYMessaging feature
    config.deviceID = CLYOpenUDID;
    [Countly.sharedInstance startWithConfig:config];
    [Countly.sharedInstance setNewDeviceID:CLYIDFA onServer:YES];

which should convert all current users over to using an IDFA, then in a few months I can remove OpenUDID altogether.

Thanks for the help.

erkanyildiz commented 8 years ago

You are welcome. Now your code looks fine. You are good to go.

Thanks to you, we noticed that we need to improve our documentation for v16.02.

Soon I will be working on documentation: Add APM Add ViewTracking Update CustomEvents Update UserDetails Update CrashReporting Update PushNotifications Merge WatchOS2

While I am working on one of those I will make "Starting Configuration" and "Changing Device ID" parts more clear.

Thanks again.

ddaddy commented 8 years ago

No problem. Just 1 more question about startup. How do we start with test messaging? Previously we had - (void)startWithTestMessagingUsing:(NSString *)appKey withHost:(NSString *)appHost andOptions:(NSDictionary *)options

ddaddy commented 8 years ago

Do you have a way to start as a test APN device? I tried the old - (void)startWithTestMessagingUsing:(NSString *)appKey withHost:(NSString *)appHost andOptions:(NSDictionary *)options method, but it goes back to crashing like before.

ddaddy commented 8 years ago

Never mind, it seems to work as a test APN device automatically :)