checkForUpdates() with dev-app-update.yml returns null result instead of expected object

[Nantris]

• Electron-Builder Version: 22.14.13

• Node Version: 14.17.6

• Electron Version: 17.4.0
• Electron Type (current, beta, nightly): current

• Target: Unpackaged/DEV (but running on Windows)

We have a code like below and I've confirmed the path is correct and that the dev-app-update.yml points to our S3 bucket, but the updateCheckResult is always null. Shouldn't we be receiving the proper versionInfo and updateInfo in an object?

I've already confirmed that the updater works outside of DEV, so why null result in DEV? This did not used to be the behavior but I don't know when it changed. We haven't touched our code or changed our update bucket path in ages.

autoUpdater.updateConfigPath = path.join(
    __dirname,
    'dev-app-update.yml'
);

autoUpdater
    .checkForUpdates()
    .then(updateCheckResult => {
        handleUpdateCheckResult(updateCheckResult);
    })

[Nantris]

Additionally I see this in my logs, despite never actually calling checkForUpdatesAndNotify. It's also not a firewall issue.

Skip checkForUpdatesAndNotify because application is not packed Updater can connect: false

[DogFoxX]

I also get Skip checkForUpdatesAndNotify because application is not packed although I'm calling autoUpdater.checkForUpdates(). I have dev-app-update.yml in root, and running

function isDev() {
   return !app.isPackaged;
};

if (isDev()) {
   autoUpdater.checkForUpdates();
};

doesn't change a thing.

Currently on v5.0.1, didn't have this issue previously on v4.6.5. I really don't want to package my app every time just to test updates.

[Nantris]

Looks like I tested originally with version 5.0.0. It's still an issue if we manually upgrade to the latest as well (5.0.4.)

@develar any thoughts on this?

[mmaietta]

Yes, I see that behavior specified here: https://github.com/electron-userland/electron-builder/blob/db0754805b4d6c5d8a4d86af7cb107db87bda303/packages/electron-updater/src/AppUpdater.ts#L252-L255

I wasn't aware that previously it could still be used locally without packaging the app. The isUpdaterActive check was moved into checkForUpdates due to users reporting error logs that it was checking for a non-existent update file. This was most notable for any distributable that doesn't support auto-update

Not sure what we should do. Seems like the warning log message needs to be updated, and it seems reasonable that we allow dev-app-update.yml to be usable. Can we use a different flag, or add an arg to checkForUpdates(forceDev = false) { ... } with default false?

[Nantris]

@mmaietta thanks very much for digging in to find the source of the issue. Have you had a chance to consider possible resolutions any further?

[Nantris]

Any update on this?

[mmaietta]

My bad, I honestly forgot about this one. I see this code is already checking isPackaged https://github.com/electron-userland/electron-builder/blob/dd29013dbd724e19e883513a123c22b2e701ad7a/packages/electron-updater/src/ElectronAppAdapter.ts#L23-L25

I'm wondering if it's better to add a property to the Updater such as forceDevUpdateConfig = false to allow bypassing the isUpdaterActive check

  public isUpdaterActive(): boolean {
    if (!this.app.isPackaged && !this.forceDevUpdateConfig) {
      this._logger.info("Skip checkForUpdates because application is not packed and dev update config check explicitly disabled")
      return false
    }
    return true
  }

Otherwise, both checkForUpdatesAndNotify and checkForUpdates have to manually pass in a new var, which doesn't seem optimized to me.

What are your thoughts

[Nantris]

Thanks very much for the update @mmaietta!

I'm maybe not the best judge of solutions since I'm not all that familiar with the rest of the code, but I think what you've laid out makes sense.

My only thought is, would it possibly make more sense to have something like this.updaterEnabled = this.app.isPackaged || this.forceDevUpdateConfig`

Again though, I'm rather oblivious to the code outside of what you've shared.