CodePush update crashes silently on iOS

[peacechen]

Description

code-push updates are causing our app to crash silently. This happens only on a release build so I'm not able to capture any debug information. If I set CheckFrequency from ON_APP_RESUME to MANUAL to stop it from automatically downloading updates, the app does not crash.

In debug mode the app does not crash. Code Push ON_APP_RESUME and explicit sync() updates work in debug mode only.

Everything has been sync'd up between the base app release update and the CodePush updates, so no native components have changed between the deployed app and the CodePush update.

CodePush was working on a previous version of our app, if that makes any difference.

Reproduction

Build a release version and run on either a real device or simulator. Using the exact same build, change one line in the JS and release it with code-push:

code-push release-react MyAppName ios --plistFile "ios/MyProject/Info.plist" -m --description "test update"

Additional Information

• react-native-code-push version: 1.16.1-beta (have also tried 1.15.1-beta and 1.17.3-beta)
• react-native version: 0.34.1
• iOS/Android/Windows version: iOS 10.2.1
• Does this reproduce on a debug build or release build? release build only
• Does this reproduce on a simulator, or only on a physical device? real device and simulator

[Amurmurmur]

I think I have a similar issue as you. I was able to catch exception in a release/staging build on ios.
Though a crash only happens on the ios platform.
#815

[peacechen]

Thanks for the reference. Can't tell whether it's the same issue since our app uses CodePush 1.15-1.17, while yours uses 2.0.2 and yours didn't have this issue on 1.17. Unless this issue is more fundamental to the CodePush platform and is triggering a bug on multiple versions.

[axemclion]

@pniko This looks like a serious issue. Could we quickly investigate to see what the issue here is ? Once @sergey-akhalkov is back to work, he could continue the investigation.

[patniko]

Sure thing. Setting up a test right now to see if I can reproduce the issue before @sergey-akhalkov becomes available again.

[didierbrun]

Hello,
It seems that the mutableUpdatePackage dictionary in CodePushPackage.m contains a "download" key that is a Function, thus it causes a crash during JSON serialization on this line :
NSData *updateSerializedData = [NSJSONSerialization dataWithJSONObject:mutableUpdatePackage options:0 error:&error];
Adding this :
[mutableUpdatePackage removeObjectForKey:@"download"];
just before the NSJSONSerialization and the crash gone.
Hope it helps.

[peacechen]

@didierbrun Thank you very much for diagnosing this. Debugging this on my end was frustrating since I had no visibility into the crash.
Would it be possible to backport this fix for React Native pre-0.40 ? (e.g. 1.17.4-beta)
We're not yet able to upgrade to the latest version of RN.

[max-mironov]

Hey Guys, just minor heads up, seems that we've already PR that should fix this issue - #809. We've started discussing issue with 'download' property here but unfortunately haven't finished yet.
In the lights of your recent answers and if you have a chance to verify if the issue is fixed by this PR I believe that we should merge it ASAP (and also enhance the fix with removing download prop on native side to be sure that it works for "Support Brownfield React Native apps" feature either).

[peacechen]

@max-mironov -- I will test that in the morning. Thanks for the link

I'm surprised that a breaking change like this isn't affecting everyone. Is there a special condition triggered by a few use cases that isn't happening for the majority of users?

[didierbrun]

@max-mironov, I will test the PR this evening, thanks for your support.

[max-mironov]

@peacechen, @didierbrun - thanks guys, please let us know the results.
I'm also a little bit surprised here because I was not able to reproduce this under typical conditions. Will try to dig deeper into this tomorrow.

[peacechen]

@max-mironov -- I've confirmed on a physical iPad that PR #809 fixes the update crash. When you attempted to recreate the bug, were you running a debug or release build? I only saw the crash on a release build. It doesn't crash on a debug build.

Would there be any way to expedite releasing this fix for the 1.x and 2.x react-native-code-push branches? Custom manual patching is hard to manage for CI.

[max-mironov]

@peacechen - just tested both debug and release builds against

    "react-native": "^0.44.0",
    "react-native-code-push": "^2.0.2-beta"

on simulator and on real device (iOS 8.1) - and it is working like a charm for me.
Also have verified for RN 0.34.1 and React-Native-CodePush 1.16.1-beta in release build and it is working well for me either.

Anyway as far as it looks like removing download property is fixing the problem I think we should go ahead and merge these PR's while we continue our investigation about the exact reproduction steps.
@axemclion, @pniko - what do you think about this?

[sergey-akhalkov]

Hello @peacechen, @Amurmurmur, @didierbrun, thank you for reaching us and for your help with the issue, and sorry for the delay. I agree with @max-mironov that we should go ahead and release the fix asap even if it does not reproduce for us (please let me know if you have any ideas or if you could share source code that can help us to reproduce it), but we also need to verify if the fix will work properly for other users who aren't experiencing the issue. So we are going to test it and make new release this days (@axemclion, @pniko, please let me know if you have other thoughts). Thank you for the patience.

[dguillamot]

Can confirm that we are also experiencing this with our app, currently in production. Looking forward to a merged fix via npm whenever you are ready to do so.

[Amurmurmur]

@sergey-akhalkov I can confirm the suggested PR #809 fixes the crash!
Cant wait to update via NPM 👍

[peacechen]

Hopefully the Microsoft team will approve the update soon. This has been a hard blocker for our app since early last week. I suspect that this is affecting a fair amount of people, with only a few who have made their way here to report it. Given the careful and minimal code changes, the risk is low, and if there is a side effect, users have the option to revert to 1.17-3 or 2.0.2.

[patniko]

Sorry for the delay on this one! PR has been merged and the latest npm published. Let us know if you continue having any issues your end with the new package.

[peacechen]

Would you mind backporting the fix to the 1.x releases? We're not able to upgrade to RN 0.4x yet.

[patniko]

@max-mironov @sergey-akhalkov Can either of you take care of the backport request today?

[peacechen]

Thanks @max-mironov @pniko

Since npm doesn't allow publishing a back-rev release, I went ahead and created a 1.17.4-beta branch in my fork, cherry-picking the relevant commits since 1.17.3-beta. It may be consumed in the package.json as:

  "dependencies": {
    ...
    "react-native-code-push": "https://github.com/peacechen/react-native-code-push#4377312",
    ...
  }

Not intended as a long-term solution, but we needed to get the fix for our app out there yesterday.

[max-mironov]

@peacechen - we wholeheartedly agree that we should provide fix also for older versions.
I've just published fix for v1.17.4, please verify (followed similar approach as here)

As far as 1.17.4 is now available via npm (as it is shown via npm view react-native-code-push) - I believe we can close the issue going further.

@peacechen - please let us know if it works for you or if you see any issues.

[peacechen]

That looks good @max-mironov . Thanks again for your help throughout all this.

[max-mironov]

Thank all of you guys for your help with this issue. I'm closing this now, please let us know if you still have any problems with this.