Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Native crash on iOS when specifying deploymentKey without config.xml entry #108

Closed
Justin-Credible opened this issue Apr 28, 2016 · 7 comments
Labels

Comments

@Justin-Credible
Copy link

I'm using the codePush.sync(...) method with SyncOptions to pass my deploymentKey. I do not have a deployment key in my config.xml file.

This causes a crash on iOS the first time the app is launched. The crash occurs in CodePushReportingManager.m line 33 when it attempts to insert a nil entry (the deployment key) into a dictionary:

 *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[1]'

Walking up the call stack to -[CodePush updateSuccess:] in CodePush.m we can see, on line 64, that the deployment key is pulled from config.xml directly (which is null in my case). I would expect this to use the value I passed into the sync method.

This is similar to #89, but instead of errors in the console, a native code crash occurs. I can use the workaround listed there to bypass the crash, but the deployment key would still be incorrect (it would be empty string instead of nil).

Version: v1.7.0-beta

Steps to reproduce:

  1. Ensure the app is NOT installed on the phone
  2. Make sure the deployment key is not in config.xml
  3. Add a call to codePush.sync(...) that passed SyncOptions with a deploymentKey set
  4. Launch the app

Expected Results: No crash and the updateSuccess method uses the deployment key I specified.

Actual Results: Native code crash because the deployment key is not present in config.xml

@chrisvasz
Copy link

I'm seeing this also.

@lostintangent
Copy link
Member

Hey guys! We can repro this bug, and as you mentioned, the workaround in the meantime is to simply add a deployment key to your config.xml. The key that you specify to the call to sync will be the one that is used, so everything should work as expected. It's a non-ideal solution, but it should be impactless on your app. Let us know if that unblocks you.

In parallel, we'll investigate a fix for this issue and try to get it out ASAP! Thanks for reporting this.

@Justin-Credible
Copy link
Author

After looking at the code a little more, it appears that the two areas that use the hardcoded deployment key from config.xml are just using it to report the status back to the server via the [CodePushReportingManager reportStatus:...] method.

Using the workaround, I've done some testing and I was able to deploy updates to staging and have the app receive them without any further issues. If anyone ends up using this workaround just remember that the usage status may not be updated correctly until this is fixed.

Thanks for the quick response!

@lostintangent
Copy link
Member

You are correct :) we've got a fix for this right now and am just running through the tests.

@lostintangent
Copy link
Member

This has been resolved with v1.7.1-beta that we just released to NPM. Thanks again for reporting this and let us know if you still hit this issue.

@Justin-Credible
Copy link
Author

Thanks for the fix!

It looks like #110 is what you are referring to. While this fixes the crash by returning early, it means that reporting does not work.

Please correct me if I'm wrong about that. If not, I would like to create an issue for it, as the reporting feature is desirable.

@lostintangent
Copy link
Member

lostintangent commented May 6, 2016

@Justin-Credible Sorry for the late reply! You are definitely right. We corrected the crash, but the reporting feature still needs to be adjusted to accommodate apps not specifying their deployment keys in the config.xml. If you could open an issue for that, that would be awesome, and we could look at that next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants