-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CodePush update crashes silently on iOS #816
Comments
I think I have a similar issue as you. I was able to catch exception in a release/staging build on ios. |
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. |
@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. |
Sure thing. Setting up a test right now to see if I can reproduce the issue before @sergey-akhalkov becomes available again. |
Hello, |
@didierbrun Thank you very much for diagnosing this. Debugging this on my end was frustrating since I had no visibility into the crash. |
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. |
@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? |
@max-mironov, I will test the PR this evening, thanks for your support. |
@peacechen, @didierbrun - thanks guys, please let us know the results. |
@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. |
`download` key is a function and isn't serializable by NSJSONSerialization. See issue microsoft#816.
@peacechen - just tested both debug and release builds against
on simulator and on real device (iOS 8.1) - and it is working like a charm for me. Anyway as far as it looks like removing |
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. |
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. |
@sergey-akhalkov I can confirm the suggested PR #809 fixes the crash! |
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. |
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. |
Would you mind backporting the fix to the 1.x releases? We're not able to upgrade to RN 0.4x yet. |
`download` key is a function and isn't serializable by NSJSONSerialization. See issue microsoft#816.
@max-mironov @sergey-akhalkov Can either of you take care of the backport request today? |
Thanks @max-mironov @pniko Since npm doesn't allow publishing a back-rev release, I went ahead and created a
Not intended as a long-term solution, but we needed to get the fix for our app out there yesterday. |
@peacechen - we wholeheartedly agree that we should provide fix also for older versions. As far as 1.17.4 is now available via npm (as it is shown via @peacechen - please let us know if it works for you or if you see any issues. |
That looks good @max-mironov . Thanks again for your help throughout all this. |
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. |
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
fromON_APP_RESUME
toMANUAL
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:
Additional Information
The text was updated successfully, but these errors were encountered: