Skip to content
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

app rolls back to the version before update after closed and opened again #48

Open
metalllus opened this issue Sep 4, 2021 · 35 comments

Comments

@metalllus
Copy link

metalllus commented Sep 4, 2021

I am testing this on an Ionic/Capacitor/Angular/Firebase project, NOT Cordova.
What I have done so far:

Created new app with appcenter apps create -d <appDisplayName> -o iOS -p Cordova
Created new deployment with appcenter codepush deployment add -a <ownerName>/<appName> Production
Built new app version ready for release with ionic capacitor build
Released new version with appcenter codepush release -a <ownerName>/<appName> -c ./PATH_TO_BUILD -m -t 1.0 -d Production
In the angular app I have this:

codePush
      .sync(
        {
          installMode: InstallMode.ON_NEXT_RESUME,
          mandatoryInstallMode: InstallMode.IMMEDIATE,
          onSyncStatusChanged: (status) => {
            console.log(status)
          },
        },
        (progress) => {
          console.log(progress)
          );
        }
      )
      .then((status) => {
        console.log(status)
      });

If I make an update available via Codepush, the update downloads and installs, the app restarts, I can see the changes, everything works.
But if I close the app and open it again, it loads in the state before the update was applied.
Tested on emulator and real device. Same behavior.
If anyone has a suggestion where I got it wrong, please do let me know.

*UPDATE: Today I tested also on Android, behavior is exactly the same as on iOS.
*UPDATE 2: Another thing that does not work for me are the callbacks that should be returning the update download progress and onSyncStatusChanged. Even though the update gets downloaded and installed these callbacks never seem to fire.

@metalllus
Copy link
Author

metalllus commented Sep 7, 2021

@leo6104 @o-alexandrov @Clovel could you help me out with this please? I desperately need to get it working...

@PIXPOINT
Copy link

PIXPOINT commented Sep 19, 2021

Same problem. The app instaliert itself, but after restarting it resets itself.

EDIT: solved by using codePush.notifyApplicationReady()

@metalllus
Copy link
Author

@PIXPOINT Yep, that did help thx. Did you manage to get the downloadProgress callback working? For me it does not fire at all.

@PIXPOINT
Copy link

PIXPOINT commented Sep 20, 2021

Yes:

remotePackage.download().then((localPackage) => {
    localPackage.install({
        installMode: InstallMode.ON_NEXT_RESUME,
        minimumBackgroundDuration: 120,
        mandatoryInstallMode: InstallMode.IMMEDIATE
    }).then(onInstallSuccess, onError);
});

@Clovel
Copy link

Clovel commented Sep 20, 2021

Hi @metalllus @PIXPOINT,

I do not have any insights on this issue for now, but we might have this issue too. I'll look into it in a few days to see if I can reproduce this issue and to pinpoint where it comes from.

@PIXPOINT, you said that you solved your issue using codePush.notifyApplicationReady(), but then showed us code without that function. Can you be more specific on the steps you took to solve your issue please ?

@PIXPOINT
Copy link

PIXPOINT commented Sep 20, 2021

Hi @Clovel @metalllus,

far down in the README I found the following:
https://github.com/mapiacompany/capacitor-codepush#codepushnotifyapplicationready

After successfully installing the update, I run codePush.notifyApplicationReady(). Also before I check whether a new update is available. That solved the reset. The way I understood it, it takes care of it. The CodePush knows that the update was successfully installed.

Here's the code I'm using and it works. Maybe someone can update the readme?

let onError = (error) => {
    console.log("Error,", error);
};

let onInstallSuccess = () => {
    
    console.log("Installation succeeded.");

    /*
        Time delay, otherwise the installation fails for me at the next start.
    */
    setTimeout(() => {
        codePush.restartApplication();
    }, 200);
    
};

let onPackageDownloaded = (localPackage) => {
    
    console.log("Downlaod succeeded.", localPackage);
    
    localPackage.install({
        installMode: InstallMode.ON_NEXT_RESUME,
        minimumBackgroundDuration: 0,
        mandatoryInstallMode: InstallMode.IMMEDIATE
    }).then(onInstallSuccess, onError);
    
};

let onUpdateCheck = (remotePackage) => {
    if (!remotePackage) {
        
        console.log("The application is up to date.");
        
        // NOTIFY CODE PUSH
        codePush.notifyApplicationReady();
        
    } else {
        if (!remotePackage.failedInstall) {
            
            console.log("A CodePush update is available. Package hash: ", remotePackage);
            
            // DOWNLOAD UPDATE
            remotePackage.download().then(onPackageDownloaded).catch(onError);
            
        } else {
            
            console.log("The available update was attempted before and failed.");
            
        }
    }
};

codePush.checkForUpdate(onUpdateCheck, onError);

@Clovel
Copy link

Clovel commented Sep 20, 2021

Ok, interesting stuff. I have a few questions for you @PIXPOINT.

  • Do you need to restart even for mandatory updates (configured w/ mandatoryInstallMode: InstallMode.IMMEDIATE in you code) ? Or does this apply only to the non mandatory updates (configured w/ installMode: InstallMode.ON_NEXT_RESUME)
  • Any reason why you d'ont use the codePush.sync method ? It is specified in the README.md that sync calls codePush.notifyApplicationReady(); for us. Perhaps this sync method doesn't work as intended.
  • How/chen does your code manage rollbacks ?

@metalllus
Copy link
Author

metalllus commented Sep 20, 2021

@Clovel I can answer with experience from my testing so far.

  1. Yes, you always need to restart after the update has been installed. No matter if the update is mandatory or not. The only reason why the mandatory flag exists in my opinion is so that you can choose a different install mode/install strategy for each option based on your preferences.
  2. a 3. That is the thing. codepush.sync() does not work as it should in my opinion. It should run codePush.notifyApplicationReady() after the update is installed and app restarts but for some reason it doesn't. You have to run it manually after the restart to confirm that the install was successful so that the update does not roll back to the previous version on next launch. I can confirm this does work.

@PIXPOINT The codePush.notifyApplicationReady() should be called (as per the docs) after the restart occurs NOT before. That is because if something gets corrupted on install and the app does not load you will end up with a bricked app if you call it before restart.

The only thing that does not work for me at the moment is the downloadProgress callback (remotePackage.download((downloadProgress) => console.log(downloadProgress.receivedBytes))) If anyone manages to get it working I think we are done here.

@PIXPOINT
Copy link

@Clovel as with @metalllus, codepush.sync() doesn't work for me either. Therefore my modified code. In my app, information is displayed for the user during the various processes (not included in my post).
I adjusted the code post so that codePush.notifyApplicationReady() is only executed after the start.
For the rollback. If the installation fails, the re-installation is blocked by remotePackage.failedInstall.

@Clovel
Copy link

Clovel commented Sep 20, 2021

I believe the nominal flow of sync should be :

  • Check if there is an update
  • If there is an update available
    • Download the update
    • Install the update
    • Notify the app that the update has been successful
    • If the install mode for this update is IMMEDIATE
      • restart the app.
    • Else
      • Wait for the next app restart (depending on the install mode of course)
  • Else
    • If an update been downloaded previously (before app start)
      • If it was successful
        • call codePush.notifyApplicationReady().

The sync method should restart if the InstallMode requires it to.
Personally, I always set the install mode to IMMEDIATE.

@metalllus are you saying that the "post-(re)start" actions aren't executed ?

Also :

If anyone manages to get it working I think we are done here.

It doesn't work for me either.

Nevertheless, we should find a way to fix codePush.sync.

@metalllus
Copy link
Author

@Clovel what I am saying is that codePush.notifyApplicationReady() is not executed automatically if you use codePush.sync() to update even if the update download and install is successful. It should run automatically after app restart as per the docs but for me it does not. Hence the problem with the app rolling back to previous version if you do not run the codePush.notifyApplicationReady() manually after the restart. It should be fixed, yes, definetely but it is not a deal breaker for me as it can be worked around.

The only thing that bugs me is the downloadProgress callback (remotePackage.download((downloadProgress) => console.log(downloadProgress.receivedBytes))) not working. It should fire repeatedly during the download process and give you totalBytes and receivedBytes on the downloadProgress object. I need this to display the download progress indicator to the user.

@Clovel
Copy link

Clovel commented Sep 20, 2021

Seems that the notifyApplicationReady method is called in syncInternal, called by sync. Take a look here : https://github.com/mapiacompany/capacitor-codepush/blob/36e607c659948ef66effffb8e38c2dae8a90cab9/src/codePush.ts#L388

This line is not called if :

  • CodePush.SyncInProgress === true
  • There is a JS error before this line

When using sync, did you get any kind of errors @metalllus ?

@metalllus
Copy link
Author

@Clovel I sent you the console output from Xcode on your gmail address.

@Clovel
Copy link

Clovel commented Sep 20, 2021

Perhaps notifyApplicationReady is called too late...

Another possibilty would be that the native code doesn't handle the notifyApplicationReady call.

Here is the code (for iOS) of notifyApplicationReady's native implemntation :
https://github.com/mapiacompany/capacitor-codepush/blob/36e607c659948ef66effffb8e38c2dae8a90cab9/ios/Plugin/CodePush.m#L143

Whatever happens, this function ends w/

    // Mark the update as confirmed and not requiring a rollback
    [CodePushPackageManager clearInstallNeedsConfirmation];
    [CodePushPackageManager cleanOldPackage];
    [call resolve];

@Clovel
Copy link

Clovel commented Sep 20, 2021

Honestly I'm not sure what is happening here.

It should run codePush.notifyApplicationReady() after the update is installed and app restarts but for some reason it doesn't.

It really seems like sync DOES call notifyApplicationReady... 🤔

@Clovel
Copy link

Clovel commented Sep 21, 2021

I'm testing putting the notifyApplicationReady at the end of the sync function. Also I'm awaiting & try/catching it. It seems to make more sense this way. If it works, I'll submit a PR.

@tolutronics
Copy link

@Clovel were you able to solve the notifyApplicationready() issue ? ... My updates are rolling back

@metalllus
Copy link
Author

@tolutronics yes, it was solved but this discussion is mute since Cordova was discontinued by Microsoft App Center recently.

@alexcroox
Copy link

alexcroox commented Nov 4, 2021

@metalllus that doesn't impact us or this plugin since we aren't using the cordova client SDK. It continues to work fine in Capacitor v3 (I'm using Vue2), just choose React Native on the codepush site and away you go.

@tolutronics just always call codePush.notifyApplicationReady() on app startup and before codePush.sync and it works fine for me on iOS and Android

@metalllus
Copy link
Author

@alexcroox man, that is good news I did not know that. thank you for pointing this out to me.

@Clovel
Copy link

Clovel commented Nov 4, 2021

@tolutronics just always call codePush.notifyApplicationReady() on app startup and before codePush.sync and it works fine for me on iOS and Android

Why before ? I had the feeling that it should be called AFTER sync, but I might be mistaken.

@alexcroox
Copy link

alexcroox commented Nov 4, 2021

@Clovel from my Cordova days of using Codepush years back I remember reading somewhere that you should call it as early as possible to inform the SDK that the install was successful. Seeing your comments above it seems that maybe it's called too late in the sync() process causing some race conditions/rollbacks. Whereas AFAIK there is no harm in calling it independently and as soon as possible at boot time so I do. No issues so far, Apple only approved the OTA updates in my new Capacitor app last week so I don't have a ton of history with this plugin to back that up yet, but my tests during development showed no rollbacks.

@Clovel
Copy link

Clovel commented Nov 4, 2021

Ok. Nice to hear. That means oe of my MRs is probably not valid. I'll do some testing and edit it.

EDIT : I actually didn't publish that branch.

@alexcroox
Copy link

@Clovel you've done a ton of work on moving this lib forward, have you heard anything from @leo6104 recently? Seems like there are people ready to keep pushing this lib forward but the only person with write access hasn't reviewed your PRs in months, nor responded to: #60

Time for a new fork/repo maybe?

@Clovel
Copy link

Clovel commented Nov 4, 2021

Well, I did get an invite that I haven't accepted yet. I still do not know if I'll have the time do maintain this repository or a fork in the future.

We'll have to discuss this w/ my collegues if we will continue to use CodePush for our capacitor apps. For now, this path has only slowed us down. Now that MS has discontinued CodePush for Cordova and that there is no indication that Capacitor will be supported anytime soon, I do not know if CodePush is a reliable solution for production apps.

If we choose to continue using CosePush, then I will likely accept a maintainer invite or invite people to use our own fork (that already exists BTW, but it's a mess for now as it's only for testing & tweaking.).

@alexcroox
Copy link

alexcroox commented Nov 4, 2021

Thanks, yeah definitely wasn't suggesting you manage it solo, and I personally don't need anything further from this repo other than future bug fixes / making it easier for others to setup, as it's doing it's job fine for me in production currently. Just noticed you had a lot of outstanding PRs that might help others and hated to see them go to waste.

MS haven't announced any plans to retire their react-native support for Codepush which is my what Capacitor/Vue2 project now uses, so I think we are fine for the time being. Looks like others might be putting effort into replicating the APIs required to use self hosted backends too which is great to see.

@Clovel
Copy link

Clovel commented Nov 4, 2021

What would be your estimate on the amount of work that needs to be done for this lib to be at least reliable & usable ? Nothing extraordinary.

Also, could you explain what exactly you did in your app to have it working ? Where do you call the different CodePush functions in your app's lifecycle ? What callbacks have you given to the CodePush sync function ?

I'd like to see what is left to do and how much of a time investment it would be to get this lib to be reliably used in production.

@metalllus
Copy link
Author

also if/how you managed to get the downloadProgress callback working... I am not able to get this callback firing at all.

@Clovel
Copy link

Clovel commented Nov 4, 2021

Also, MS discontinued Cordova, but existing projects don't seem to be affected. The proposed command line is still

appcenter codepush release-cordova -a <YOUR_PROJECT>/<YOUR_APP> -d <ENVIRONMENT>

Shouldn't it be just release ?

@Clovel
Copy link

Clovel commented Nov 4, 2021

also if/how you managed to get the downloadProgress callback working... I am not able to get this callback firing at all.

Sorry @metalllus, I didn't get the time to look at that yet...

@Clovel
Copy link

Clovel commented Nov 4, 2021

One thing that needs to be done is to have a good build process to publish this package to NPM.

For now, the build products are versionned on git.

It should be built by CI/CD plans and published afterwards.

@alexcroox
Copy link

alexcroox commented Nov 4, 2021

This is basically all I'm doing in a Nuxt plugin which gets called pretty early in the JS app startup lifecyle:

Ignore the imports and the reason I'm await importing is simply to stop any of this lib loading on the web version.

import { syncStatus } from '~/lib/enums/over-the-air-updates-sync-status'
import { isNative, platform } from '~/plugins/native/capacitor'

// Initialise over the air updates from MS Codepush
export default async function ({ store, app, $log }) {
  if (isNative) {
    const { codePush, InstallMode } = await import('capacitor-codepush')

    codePush.notifyApplicationReady()

    const otaUpdates = {
      sync: async () => {
        await codePush.sync({
          deploymentKey:
            platform === 'ios' ? process.env.CODE_PUSH_IOS_DEPLOY_KEY : process.env.CODE_PUSH_ANDROID_DEPLOY_KEY,
          installMode: InstallMode.ON_NEXT_RESUME,
          mandatoryInstallMode: InstallMode.IMMEDIATE,
          onSyncStatusChanged: statusCode => {
            $log.debug('OTA Update: Status changed', syncStatus[statusCode])
            store.commit('over-the-air-updates/setStatusCode', statusCode)
          },
          onSyncError: error => {
            $log.error('OTA Update: Error', error)
            store.commit('over-the-air-updates/setError', error)
          }
        })
      }
    }

    otaUpdates.sync()

    const { App } = await import('@capacitor/app')

    App.addListener('appStateChange', appState => {
      if (appState.isActive) {
        otaUpdates.sync()
      }
    })
  }
}

Capacitor config:

plugins: {
    CodePush: {
      SERVER_URL: 'https://codepush.appcenter.ms/'
    }

To publish releases I'm doing:

appcenter codepush release --app ProjectName/appname-ios -c './ios/App/App/public/' --target-binary-version '^3' --deployment-name production

For android and ios in the appcenter website I have setup react-native projects (even though I'm not using React, it doesn't matter)

image

@metalllus I'm not using downloadProgress at all because Apple will reject your app if it contains alternative update mechanisms. Which means you can't show update dialogs or download progresses. For me I approach it a little more discretely on the footer where my version number is and I use 3 letter codes to indicate the status of codepush:

CHK = checking for updates
UTD = up to date, no new updates to install
UDL = update downloading
INS = update installed, waiting for the app to lose focus, then regain it to finish up

(not helpful for the end user but useful for me to debug any issues they might be having)

image

@metalllus
Copy link
Author

@alexcroox I read about that but could you please explain to me how they check for that? Because if I submit the app for review with the latest code then the download progress/update available window will not be displayed to the reviewer from Apple so how exactly do they find out?

@alexcroox
Copy link

alexcroox commented Nov 4, 2021

They may not, I have no idea what process they take or how closely they look or auto scan your submitted code. You might get away with it, you might not, for me it wasn't worth the risk. My fastlane automations make it trivial to submit to Apple/Google at the same time I submit to codepush so my OTA updates only benefit the regular users or those who would benefit an immediate bug fix rather than waiting a day or 2 for store approval.

@LouiMinister
Copy link

LouiMinister commented Dec 21, 2021

I got a solution on this issue.

...
 const status = await codePush.sync(syncOptions);
        switch (status) {
          case SyncStatus.UPDATE_INSTALLED: this.log('SyncStatus.UPDATE_INSTALLED');
            codePush.notifyApplicationReady();
            break;
         }
...

Like this code, after invoke sync, whenstatus is UPDATE_INSTALLED, call notifyApplicationReady;

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

No branches or pull requests

6 participants