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

refactor: change quitAndInstall to asynchronous to prevent blocking main thread while updating #8405

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/perfect-pumpkins-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"electron-updater": major
---

refactor: change quitAndInstall to asynchronous to prevent blocking main thread while updating
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,11 @@ module.exports = {
},
],
"@typescript-eslint/no-redundant-type-constituents": "off",
"@typescript-eslint/no-misused-promises": [
"error",
{
"checksVoidReturn": false
}
]
},
}
60 changes: 30 additions & 30 deletions docs/api/electron-builder.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/electron-updater/src/AppAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface AppAdapter {

quit(): void

onQuit(handler: (exitCode: number) => void): void
onQuit(handler: () => Promise<void>): void
}

export function getAppCacheDir() {
Expand Down
15 changes: 9 additions & 6 deletions packages/electron-updater/src/AppImageUpdater.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
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"
import { FileWithEmbeddedBlockMapDifferentialDownloader } from "./differentialDownloader/FileWithEmbeddedBlockMapDifferentialDownloader"
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)
Expand Down Expand Up @@ -73,7 +76,7 @@ export class AppImageUpdater extends BaseUpdater {
})
}

protected doInstall(options: InstallOptions): boolean {
protected async doInstall(options: InstallOptions): Promise<boolean> {
const appImageFile = process.env["APPIMAGE"]!
if (appImageFile == null) {
throw newError("APPIMAGE env is not defined", "ERR_UPDATER_OLD_FILE_NOT_FOUND")
Expand All @@ -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)
}
Expand All @@ -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
}
Expand Down
96 changes: 69 additions & 27 deletions packages/electron-updater/src/BaseUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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
Expand All @@ -38,10 +38,10 @@ export abstract class BaseUpdater extends AppUpdater {
}

// must be sync
protected abstract doInstall(options: InstallOptions): boolean
protected abstract doInstall(options: InstallOptions): Promise<boolean>

// must be sync (because quit even handler is not async)
install(isSilent = false, isForceRunAfter = false): boolean {
async install(isSilent = false, isForceRunAfter = false): Promise<boolean> {
if (this.quitAndInstallCalled) {
this._logger.warn("install call ignored: quitAndInstallCalled is set to true")
return false
Expand Down Expand Up @@ -89,7 +89,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
Expand All @@ -100,20 +100,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)
Expand Down Expand Up @@ -143,23 +138,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<boolean> {
this._logger.info(`Executing: ${cmd} with args: ${args}`)
return new Promise<boolean>((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 }
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/electron-updater/src/DebUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ export class DebUpdater extends BaseUpdater {
})
}

protected doInstall(options: InstallOptions): boolean {
const sudo = this.wrapSudo()
protected async doInstall(options: InstallOptions): Promise<boolean> {
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"]
Expand Down
35 changes: 32 additions & 3 deletions packages/electron-updater/src/ElectronAppAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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()
}
Expand Down Expand Up @@ -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>): 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()
})
}
}
48 changes: 29 additions & 19 deletions packages/electron-updater/src/NsisUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
const args = ["--updated"]
if (options.isSilent) {
args.push("/S")
Expand All @@ -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<void> => {
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
}

Expand Down
10 changes: 5 additions & 5 deletions packages/electron-updater/src/RpmUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@ export class RpmUpdater extends BaseUpdater {
})
}

protected doInstall(options: InstallOptions): boolean {
protected async doInstall(options: InstallOptions): Promise<boolean> {
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()
}
Expand Down
Loading