From d6031ee0a14656d98a6089690cf3b75c1092d4b1 Mon Sep 17 00:00:00 2001 From: xyloflake Date: Wed, 7 Aug 2024 19:40:36 +0530 Subject: [PATCH 1/6] fix: handle spaces for all linux platforms instead of only .deb --- packages/electron-updater/src/BaseUpdater.ts | 15 ++++++++++++++- packages/electron-updater/src/DebUpdater.ts | 5 +---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/electron-updater/src/BaseUpdater.ts b/packages/electron-updater/src/BaseUpdater.ts index 4851435ced0..5a8e35c7269 100644 --- a/packages/electron-updater/src/BaseUpdater.ts +++ b/packages/electron-updater/src/BaseUpdater.ts @@ -48,7 +48,20 @@ export abstract class BaseUpdater extends AppUpdater { } const downloadedUpdateHelper = this.downloadedUpdateHelper - const installerPath = downloadedUpdateHelper == null ? null : downloadedUpdateHelper.file + + // Get the installer path, ensuring spaces are escaped on Linux + // 1. Check if downloadedUpdateHelper is not null + // 2. Check if downloadedUpdateHelper.file is not null + // 3. If both checks pass: + // a. If the platform is Linux, replace spaces with '\ ' for shell compatibility + // b. If the platform is not Linux, use the original path + // 4. If any check fails, set installerPath to null + const installerPath = downloadedUpdateHelper && downloadedUpdateHelper.file + ? (process.platform === 'linux' + ? downloadedUpdateHelper.file.replace(/ /g, '\\ ') + : downloadedUpdateHelper.file) + : null; + const downloadedFileInfo = downloadedUpdateHelper == null ? null : downloadedUpdateHelper.downloadedFileInfo if (installerPath == null || downloadedFileInfo == null) { this.dispatchError(new Error("No valid update available, can't quit and install")) diff --git a/packages/electron-updater/src/DebUpdater.ts b/packages/electron-updater/src/DebUpdater.ts index 17b60e0663a..19705a72ea2 100644 --- a/packages/electron-updater/src/DebUpdater.ts +++ b/packages/electron-updater/src/DebUpdater.ts @@ -31,10 +31,7 @@ export class DebUpdater extends BaseUpdater { const sudo = this.wrapSudo() // pkexec doesn't want the command to be wrapped in " quotes const wrapper = /pkexec/i.test(sudo) ? "" : `"` - // application artifact names may include spaces in their name which leads - // to an error when the install command is executed - const installerPath = options.installerPath.replace(/ /g, "\\ ") - const cmd = ["dpkg", "-i", installerPath, "||", "apt-get", "install", "-f", "-y"] + const cmd = ["dpkg", "-i", options.installerPath, "||", "apt-get", "install", "-f", "-y"] this.spawnSyncLog(sudo, [`${wrapper}/bin/bash`, "-c", `'${cmd.join(" ")}'${wrapper}`]) if (options.isForceRunAfter) { this.app.relaunch() From a79b9904849c299e6e6c717d0582f6d43f779675 Mon Sep 17 00:00:00 2001 From: xyloflake Date: Wed, 7 Aug 2024 19:56:02 +0530 Subject: [PATCH 2/6] Add changeset --- .changeset/old-ducks-cry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/old-ducks-cry.md diff --git a/.changeset/old-ducks-cry.md b/.changeset/old-ducks-cry.md new file mode 100644 index 00000000000..623f8dfcb30 --- /dev/null +++ b/.changeset/old-ducks-cry.md @@ -0,0 +1,5 @@ +--- +"electron-updater": minor +--- + +Handle spaces for all linux package managers From c8ce5a86c0eaaa77158c7f159ac49c30bbeac773 Mon Sep 17 00:00:00 2001 From: xyloflake Date: Thu, 8 Aug 2024 02:11:12 +0530 Subject: [PATCH 3/6] refactor: change quitAndInstall to be asynchronous modified quitAndInstall from synchronous to asynchronous to prevent blocking the event loop during updates. This change enhances the application's responsiveness by allowing other tasks to proceed while the update is being processed. BREAKING CHANGE: The quitAndInstall function is now asynchronous, which may require updates to the calling code to handle promises. --- .eslintrc.js | 6 + packages/electron-updater/src/AppAdapter.ts | 2 +- .../electron-updater/src/AppImageUpdater.ts | 15 ++- packages/electron-updater/src/BaseUpdater.ts | 107 ++++++++++++------ packages/electron-updater/src/DebUpdater.ts | 6 +- .../src/ElectronAppAdapter.ts | 35 +++++- packages/electron-updater/src/NsisUpdater.ts | 48 ++++---- packages/electron-updater/src/RpmUpdater.ts | 10 +- 8 files changed, 160 insertions(+), 69 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index e5956b507d6..6a1cf30d240 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -42,5 +42,11 @@ module.exports = { }, ], "@typescript-eslint/no-redundant-type-constituents": "off", + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checksVoidReturn": false + } + ] }, } diff --git a/packages/electron-updater/src/AppAdapter.ts b/packages/electron-updater/src/AppAdapter.ts index 69cc8174bb7..9baf483acc6 100644 --- a/packages/electron-updater/src/AppAdapter.ts +++ b/packages/electron-updater/src/AppAdapter.ts @@ -28,7 +28,7 @@ export interface AppAdapter { quit(): void - onQuit(handler: (exitCode: number) => void): void + onQuit(handler: () => Promise): void } export function getAppCacheDir() { diff --git a/packages/electron-updater/src/AppImageUpdater.ts b/packages/electron-updater/src/AppImageUpdater.ts index a8cc747e79a..7a0ab14a173 100644 --- a/packages/electron-updater/src/AppImageUpdater.ts +++ b/packages/electron-updater/src/AppImageUpdater.ts @@ -1,8 +1,9 @@ -import { AllPublishOptions, newError } from "builder-util-runtime" -import { execFileSync } from "child_process" +import { promisify } from "util" +import { execFile as execFileCallback } from "child_process" import { chmod } from "fs-extra" import { unlinkSync } from "fs" import * as path from "path" +import { AllPublishOptions, newError } from "builder-util-runtime" import { DownloadUpdateOptions } from "./AppUpdater" import { BaseUpdater, InstallOptions } from "./BaseUpdater" import { DifferentialDownloaderOptions } from "./differentialDownloader/DifferentialDownloader" @@ -10,6 +11,8 @@ import { FileWithEmbeddedBlockMapDifferentialDownloader } from "./differentialDo import { DOWNLOAD_PROGRESS } from "./main" import { findFile } from "./providers/Provider" +const execFile = promisify(execFileCallback) + export class AppImageUpdater extends BaseUpdater { constructor(options?: AllPublishOptions | null, app?: any) { super(options, app) @@ -73,7 +76,7 @@ export class AppImageUpdater extends BaseUpdater { }) } - protected doInstall(options: InstallOptions): boolean { + protected async doInstall(options: InstallOptions): Promise { const appImageFile = process.env["APPIMAGE"]! if (appImageFile == null) { throw newError("APPIMAGE env is not defined", "ERR_UPDATER_OLD_FILE_NOT_FOUND") @@ -93,7 +96,7 @@ export class AppImageUpdater extends BaseUpdater { destination = path.join(path.dirname(appImageFile), path.basename(options.installerPath)) } - execFileSync("mv", ["-f", options.installerPath, destination]) + await execFile("mv", ["-f", options.installerPath, destination]) if (destination !== appImageFile) { this.emit("appimage-filename-updated", destination) } @@ -105,10 +108,10 @@ export class AppImageUpdater extends BaseUpdater { if (options.isForceRunAfter) { // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.spawnLog(destination, [], env) + await this.spawnLogAsync(destination, [], env) } else { env.APPIMAGE_EXIT_AFTER_INSTALL = "true" - execFileSync(destination, [], { env }) + await execFile(destination, [], { env }) } return true } diff --git a/packages/electron-updater/src/BaseUpdater.ts b/packages/electron-updater/src/BaseUpdater.ts index 5a8e35c7269..388961fa5e0 100644 --- a/packages/electron-updater/src/BaseUpdater.ts +++ b/packages/electron-updater/src/BaseUpdater.ts @@ -11,10 +11,10 @@ export abstract class BaseUpdater extends AppUpdater { super(options, app) } - quitAndInstall(isSilent = false, isForceRunAfter = false): void { + async quitAndInstall(isSilent = false, isForceRunAfter = false): Promise { this._logger.info(`Install on explicit quitAndInstall`) // If NOT in silent mode use `autoRunAppAfterInstall` to determine whether to force run the app - const isInstalled = this.install(isSilent, isSilent ? isForceRunAfter : this.autoRunAppAfterInstall) + const isInstalled = await this.install(isSilent, isSilent ? isForceRunAfter : this.autoRunAppAfterInstall) if (isInstalled) { setImmediate(() => { // this event is normally emitted when calling quitAndInstall, this emulates that @@ -38,17 +38,17 @@ export abstract class BaseUpdater extends AppUpdater { } // must be sync - protected abstract doInstall(options: InstallOptions): boolean + protected abstract doInstall(options: InstallOptions): Promise // must be sync (because quit even handler is not async) - install(isSilent = false, isForceRunAfter = false): boolean { + async install(isSilent = false, isForceRunAfter = false): Promise { if (this.quitAndInstallCalled) { this._logger.warn("install call ignored: quitAndInstallCalled is set to true") return false } const downloadedUpdateHelper = this.downloadedUpdateHelper - + // Get the installer path, ensuring spaces are escaped on Linux // 1. Check if downloadedUpdateHelper is not null // 2. Check if downloadedUpdateHelper.file is not null @@ -56,10 +56,11 @@ export abstract class BaseUpdater extends AppUpdater { // a. If the platform is Linux, replace spaces with '\ ' for shell compatibility // b. If the platform is not Linux, use the original path // 4. If any check fails, set installerPath to null - const installerPath = downloadedUpdateHelper && downloadedUpdateHelper.file - ? (process.platform === 'linux' - ? downloadedUpdateHelper.file.replace(/ /g, '\\ ') - : downloadedUpdateHelper.file) + const installerPath = + downloadedUpdateHelper && downloadedUpdateHelper.file + ? (process.platform === 'linux' + ? downloadedUpdateHelper.file.replace(/ /g, '\\ ') + : downloadedUpdateHelper.file) : null; const downloadedFileInfo = downloadedUpdateHelper == null ? null : downloadedUpdateHelper.downloadedFileInfo @@ -92,7 +93,7 @@ export abstract class BaseUpdater extends AppUpdater { this.quitHandlerAdded = true - this.app.onQuit(exitCode => { + this.app.onQuit(async () => { if (this.quitAndInstallCalled) { this._logger.info("Update installer has already been triggered. Quitting application.") return @@ -103,20 +104,15 @@ export abstract class BaseUpdater extends AppUpdater { return } - if (exitCode !== 0) { - this._logger.info(`Update will be not installed on quit because application is quitting with exit code ${exitCode}`) - return - } - this._logger.info("Auto install update on quit") - this.install(true, false) + await this.install(true, false) }) } - protected wrapSudo() { + protected async wrapSudo() { const { name } = this.app const installComment = `"${name} would like to update"` - const sudo = this.spawnSyncLog("which gksudo || which kdesudo || which pkexec || which beesu") + const sudo = (await this.spawnLogAsync("which gksudo || which kdesudo || which pkexec || which beesu")).stdout const command = [sudo] if (/kdesudo/i.test(sudo)) { command.push("--comment", installComment) @@ -146,23 +142,70 @@ export abstract class BaseUpdater extends AppUpdater { */ // https://github.com/electron-userland/electron-builder/issues/1129 // Node 8 sends errors: https://nodejs.org/dist/latest-v8.x/docs/api/errors.html#errors_common_system_errors - protected async spawnLog(cmd: string, args: string[] = [], env: any = undefined, stdio: StdioOptions = "ignore"): Promise { - this._logger.info(`Executing: ${cmd} with args: ${args}`) - return new Promise((resolve, reject) => { - try { - const params: SpawnOptions = { stdio, env, detached: true } - const p = spawn(cmd, args, params) - p.on("error", error => { - reject(error) + + protected async spawnLogAsync( + cmd: string, + args: string[] = [], + env: any = undefined, + stdio: StdioOptions = ["pipe", "pipe", "pipe"] + ): Promise<{ stdout: string; stderr: string; success: boolean }> { + this._logger.info(`Executing: ${cmd} with args: ${args.join(" ")}`) + + try { + const params: SpawnOptions = { + stdio, // Capture stdout and stderr + env: { ...process.env, ...env }, + shell: true, + detached: true, + } + + const p = spawn(cmd, args, params) + + let stdout = "" + let stderr = "" + + if (p.stdout) { + p.stdout.on("data", data => { + stdout += data.toString() }) + } + + if (p.stderr) { + p.stderr.on("data", data => { + stderr += data.toString() + }) + } + + return await new Promise<{ stdout: string; stderr: string; success: boolean }>((resolve, reject) => { + p.on("error", (error: unknown) => { + let errorMessage = "Unknown error" + if (error instanceof Error) { + errorMessage = error.message + } + reject({ stdout: "", stderr: errorMessage, success: false }) + }) + p.on("spawn", () => { + this._logger.info(`Command spawned successfully`) + }) + p.on("close", code => { + if (code === 0) { + resolve({ stdout: stdout.trim(), stderr: stderr.trim(), success: true }) + } else { + this._logger.error(`Process exited with code: ${code}, stderr: ${stderr.trim()}`) + resolve({ stdout: stdout.trim(), stderr: stderr.trim(), success: false }) + } + }) + p.unref() - if (p.pid !== undefined) { - resolve(true) - } - } catch (error) { - reject(error) + }) + } catch (error: unknown) { + let errorMessage = "Unknown error" + if (error instanceof Error) { + errorMessage = error.message } - }) + this._logger.error(`Error executing ${cmd}: ${errorMessage}`) + return { stdout: "", stderr: errorMessage, success: false } + } } } diff --git a/packages/electron-updater/src/DebUpdater.ts b/packages/electron-updater/src/DebUpdater.ts index 19705a72ea2..1223157f17e 100644 --- a/packages/electron-updater/src/DebUpdater.ts +++ b/packages/electron-updater/src/DebUpdater.ts @@ -27,12 +27,12 @@ export class DebUpdater extends BaseUpdater { }) } - protected doInstall(options: InstallOptions): boolean { - const sudo = this.wrapSudo() + protected async doInstall(options: InstallOptions): Promise { + const sudo = await this.wrapSudo() // pkexec doesn't want the command to be wrapped in " quotes const wrapper = /pkexec/i.test(sudo) ? "" : `"` const cmd = ["dpkg", "-i", options.installerPath, "||", "apt-get", "install", "-f", "-y"] - this.spawnSyncLog(sudo, [`${wrapper}/bin/bash`, "-c", `'${cmd.join(" ")}'${wrapper}`]) + await this.spawnLogAsync(sudo, [`${wrapper}/bin/bash`, "-c", `'${cmd.join(" ")}'${wrapper}`]) if (options.isForceRunAfter) { this.app.relaunch() } diff --git a/packages/electron-updater/src/ElectronAppAdapter.ts b/packages/electron-updater/src/ElectronAppAdapter.ts index a4480f8624f..b1cba64eab8 100644 --- a/packages/electron-updater/src/ElectronAppAdapter.ts +++ b/packages/electron-updater/src/ElectronAppAdapter.ts @@ -2,12 +2,21 @@ import * as path from "path" import { AppAdapter, getAppCacheDir } from "./AppAdapter" export class ElectronAppAdapter implements AppAdapter { - constructor(private readonly app = require("electron").app) {} + private app: Electron.App + + constructor(private readonly electron = require("electron")) { + this.app = this.electron.app + } whenReady(): Promise { return this.app.whenReady() } + private closeAllWindows(): void { + const windows: Electron.BrowserWindow[] = this.electron.BrowserWindow.getAllWindows() + windows.forEach(window => window.close()) + } + get version(): string { return this.app.getVersion() } @@ -40,7 +49,27 @@ export class ElectronAppAdapter implements AppAdapter { this.app.relaunch() } - onQuit(handler: (exitCode: number) => void): void { - this.app.once("quit", (_: Electron.Event, exitCode: number) => handler(exitCode)) + onQuit(handler: () => Promise): void { + this.app.once("before-quit", async (event: Electron.Event) => { + // prevent quitting + event.preventDefault() + // Close all windows before quitting + this.closeAllWindows() + // show notification that update is starting + new this.electron.Notification({ + title: `${this.app.name} is now updating`, + body: `Please don't force quit the app as it might lead to corruption.`, + }).show() + + try { + // handle auto update + await handler() + } catch (error) { + console.error(`Error during onQuit handler: ${error}`) + } + + // resume quit after handler completes + this.app.quit() + }) } } diff --git a/packages/electron-updater/src/NsisUpdater.ts b/packages/electron-updater/src/NsisUpdater.ts index 629f5e357df..6eaffb6b600 100644 --- a/packages/electron-updater/src/NsisUpdater.ts +++ b/packages/electron-updater/src/NsisUpdater.ts @@ -122,7 +122,7 @@ export class NsisUpdater extends BaseUpdater { return await this._verifyUpdateCodeSignature(Array.isArray(publisherName) ? publisherName : [publisherName], tempUpdateFile) } - protected doInstall(options: InstallOptions): boolean { + protected async doInstall(options: InstallOptions): Promise { const args = ["--updated"] if (options.isSilent) { args.push("/S") @@ -143,33 +143,43 @@ export class NsisUpdater extends BaseUpdater { args.push(`--package-file=${packagePath}`) } - const callUsingElevation = (): void => { - this.spawnLog(path.join(process.resourcesPath, "elevate.exe"), [options.installerPath].concat(args)).catch(e => this.dispatchError(e)) + const callUsingElevation = async (): Promise => { + await this.spawnLogAsync(path.join(process.resourcesPath, "elevate.exe"), [options.installerPath].concat(args)).catch(e => this.dispatchError(e)) } if (options.isAdminRightsRequired) { this._logger.info("isAdminRightsRequired is set to true, run installer using elevate.exe") - callUsingElevation() + await callUsingElevation() return true } - this.spawnLog(options.installerPath, args).catch((e: Error) => { - // https://github.com/electron-userland/electron-builder/issues/1129 - // Node 8 sends errors: https://nodejs.org/dist/latest-v8.x/docs/api/errors.html#errors_common_system_errors - const errorCode = (e as NodeJS.ErrnoException).code - this._logger.info( - `Cannot run installer: error code: ${errorCode}, error message: "${e.message}", will be executed again using elevate if EACCES, and will try to use electron.shell.openItem if ENOENT` - ) - if (errorCode === "UNKNOWN" || errorCode === "EACCES") { - callUsingElevation() - } else if (errorCode === "ENOENT") { - require("electron") - .shell.openPath(options.installerPath) - .catch((err: Error) => this.dispatchError(err)) + try { + await this.spawnLogAsync(options.installerPath, args) + } catch (e: unknown) { + // TypeScript requires catch variables to be of type 'unknown' or 'any' + // Cast to 'Error' to handle specific error properties + if (e instanceof Error) { + // https://github.com/electron-userland/electron-builder/issues/1129 + // Node 8 sends errors: https://nodejs.org/dist/latest-v8.x/docs/api/errors.html#errors_common_system_errors + const errorCode = (e as NodeJS.ErrnoException).code + this._logger.info( + `Cannot run installer: error code: ${errorCode}, error message: "${e.message}", will be executed again using elevate if EACCES, and will try to use electron.shell.openItem if ENOENT` + ) + if (errorCode === "UNKNOWN" || errorCode === "EACCES") { + await callUsingElevation() + } else if (errorCode === "ENOENT") { + await require("electron") + .shell.openPath(options.installerPath) + .catch((err: Error) => this.dispatchError(err)) + } else { + this.dispatchError(e) + } } else { - this.dispatchError(e) + // Handle cases where 'e' is not an instance of Error + this._logger.error(`Unexpected error: ${e}`) + this.dispatchError(new Error(String(e))) } - }) + } return true } diff --git a/packages/electron-updater/src/RpmUpdater.ts b/packages/electron-updater/src/RpmUpdater.ts index 201170ad783..7453e93a0ac 100644 --- a/packages/electron-updater/src/RpmUpdater.ts +++ b/packages/electron-updater/src/RpmUpdater.ts @@ -27,20 +27,20 @@ export class RpmUpdater extends BaseUpdater { }) } - protected doInstall(options: InstallOptions): boolean { + protected async doInstall(options: InstallOptions): Promise { const upgradePath = options.installerPath - const sudo = this.wrapSudo() + const sudo = await this.wrapSudo() // pkexec doesn't want the command to be wrapped in " quotes const wrapper = /pkexec/i.test(sudo) ? "" : `"` - const packageManager = this.spawnSyncLog("which zypper") + const packageManager = (await this.spawnLogAsync("which zypper")).stdout let cmd: string[] if (!packageManager) { - const packageManager = this.spawnSyncLog("which dnf || which yum") + const packageManager = (await this.spawnLogAsync("which dnf || which yum")).stdout cmd = [packageManager, "-y", "install", upgradePath] } else { cmd = [packageManager, "--no-refresh", "install", "--allow-unsigned-rpm", "-y", "-f", upgradePath] } - this.spawnSyncLog(sudo, [`${wrapper}/bin/bash`, "-c", `'${cmd.join(" ")}'${wrapper}`]) + await this.spawnLogAsync(sudo, [`${wrapper}/bin/bash`, "-c", `'${cmd.join(" ")}'${wrapper}`]) if (options.isForceRunAfter) { this.app.relaunch() } From 73b0e8cc12340c5f976acd944b92df003859c121 Mon Sep 17 00:00:00 2001 From: xyloflake Date: Thu, 8 Aug 2024 02:16:26 +0530 Subject: [PATCH 4/6] Update docs & add changeset --- .changeset/perfect-pumpkins-march.md | 5 +++ docs/api/electron-builder.md | 60 ++++++++++++++-------------- 2 files changed, 35 insertions(+), 30 deletions(-) create mode 100644 .changeset/perfect-pumpkins-march.md diff --git a/.changeset/perfect-pumpkins-march.md b/.changeset/perfect-pumpkins-march.md new file mode 100644 index 00000000000..fd75265ed28 --- /dev/null +++ b/.changeset/perfect-pumpkins-march.md @@ -0,0 +1,5 @@ +--- +"electron-updater": major +--- + +refactor: change quitAndInstall to asynchronous to prevent blocking main thread while updating diff --git a/docs/api/electron-builder.md b/docs/api/electron-builder.md index 7b4c0125196..3cfb4c1b857 100644 --- a/docs/api/electron-builder.md +++ b/docs/api/electron-builder.md @@ -3149,8 +3149,8 @@ return await getCertInfo(cscFile, cscInfo.password || "")
  • .AppImageUpdater ⇐ BaseUpdater
  • .AppUpdater ⇐ module:tiny-typed-emitter/lib/index.TypedEmitter @@ -3167,8 +3167,8 @@ return await getCertInfo(cscFile, cscInfo.password || "")
  • .BaseUpdater ⇐ AppUpdater