mapiacompany / capacitor-codepush

Capacitor plugin for CodePush
http://appcenter.ms
Other
151 stars 65 forks source link

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

Open metalllus opened 3 years ago

metalllus commented 3 years ago

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 commented 3 years ago

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

PIXPOINT commented 2 years ago

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

EDIT: solved by using codePush.notifyApplicationReady()

metalllus commented 2 years ago

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

PIXPOINT commented 2 years ago

Yes:

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

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 commented 2 years ago

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 commented 2 years ago

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

metalllus commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

I believe the nominal flow of sync should be :

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 commented 2 years ago

@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 commented 2 years ago

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 :

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

metalllus commented 2 years ago

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

Clovel commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

metalllus commented 2 years ago

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

alexcroox commented 2 years ago

@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 commented 2 years ago

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

Clovel commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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: https://github.com/mapiacompany/capacitor-codepush/issues/60

Time for a new fork/repo maybe?

Clovel commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

Clovel commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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;