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

fix: adding retries to account for dmg hdiutil flakiness #8135

Merged
merged 1 commit into from
Mar 13, 2024
Merged
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
11 changes: 11 additions & 0 deletions .changeset/fast-maps-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"app-builder-lib": patch
"builder-util": patch
"dmg-builder": patch
"electron-builder": patch
"electron-builder-squirrel-windows": patch
"electron-publish": patch
"electron-updater": patch
---

fix: unstable hdiutil retry mechanism
8 changes: 4 additions & 4 deletions packages/builder-util/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,14 @@ export async function executeAppBuilder(
}
}

export async function retry<T>(task: () => Promise<T>, retriesLeft: number, interval: number, backoff = 0, attempt = 0): Promise<T> {
export async function retry<T>(task: () => Promise<T>, retryCount: number, interval: number, backoff = 0, attempt = 0, shouldRetry?: (e: any) => boolean): Promise<T> {
try {
return await task()
} catch (error: any) {
log.info(`Above command failed, retrying ${retriesLeft} more times`)
if (retriesLeft > 0) {
log.info(`Above command failed, retrying ${retryCount} more times`)
if ((shouldRetry?.(error) ?? true) && retryCount > 0) {
await new Promise(resolve => setTimeout(resolve, interval + backoff * attempt))
return await retry(task, retriesLeft - 1, interval, backoff, attempt + 1)
return await retry(task, retryCount - 1, interval, backoff, attempt + 1, shouldRetry)
} else {
throw error
}
Expand Down
13 changes: 6 additions & 7 deletions packages/dmg-builder/src/dmg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import MacPackager from "app-builder-lib/out/macPackager"
import { createBlockmap } from "app-builder-lib/out/targets/differentialUpdateInfoBuilder"
import { executeAppBuilderAsJson } from "app-builder-lib/out/util/appBuilder"
import { sanitizeFileName } from "app-builder-lib/out/util/filename"
import { Arch, AsyncTaskManager, exec, getArchSuffix, InvalidConfigurationError, isEmptyOrSpaces, log, spawn, retry } from "builder-util"
import { Arch, AsyncTaskManager, exec, getArchSuffix, InvalidConfigurationError, isEmptyOrSpaces, log } from "builder-util"
import { CancellationToken } from "builder-util-runtime"
import { copyDir, copyFile, exists, statOrNull } from "builder-util/out/fs"
import { stat } from "fs-extra"
Expand All @@ -13,6 +13,7 @@ import { TmpDir } from "temp-file"
import { addLicenseToDmg } from "./dmgLicense"
import { attachAndExecute, computeBackground, detach, getDmgVendorPath } from "./dmgUtil"
import { release as getOsRelease } from "os"
import { hdiUtil } from "./hdiuil"

export class DmgTarget extends Target {
readonly options: DmgOptions = this.packager.config.dmg || Object.create(null)
Expand Down Expand Up @@ -51,7 +52,7 @@ export class DmgTarget extends Target {
const backgroundFile = specification.background == null ? null : await transformBackgroundFileIfNeed(specification.background, packager.info.tempDirManager)
const finalSize = await computeAssetSize(packager.info.cancellationToken, tempDmg, specification, backgroundFile)
const expandingFinalSize = finalSize * 0.1 + finalSize
await exec("hdiutil", ["resize", "-size", expandingFinalSize.toString(), tempDmg])
await hdiUtil(["resize", "-size", expandingFinalSize.toString(), tempDmg])

const volumePath = path.join("/Volumes", volumeName)
if (await exists(volumePath)) {
Expand All @@ -68,9 +69,9 @@ export class DmgTarget extends Target {
if (specification.format === "UDZO") {
args.push("-imagekey", `zlib-level=${process.env.ELECTRON_BUILDER_COMPRESSION_LEVEL || "9"}`)
}
await spawn("hdiutil", addLogLevel(args))
await hdiUtil(addLogLevel(args))
if (this.options.internetEnabled && parseInt(getOsRelease().split(".")[0], 10) < 19) {
await exec("hdiutil", addLogLevel(["internet-enable"]).concat(artifactPath))
await hdiUtil(addLogLevel(["internet-enable"]).concat(artifactPath))
}

const licenseData = await addLicenseToDmg(packager, artifactPath)
Expand Down Expand Up @@ -210,9 +211,7 @@ async function createStageDmg(tempDmg: string, appPath: string, volumeName: stri
}
imageArgs.push("-fs", ...filesystem)
imageArgs.push(tempDmg)
// The reason for retrying up to ten times is that hdiutil create in some cases fail to unmount due to "resource busy".
// https://github.com/electron-userland/electron-builder/issues/5431
await retry(() => spawn("hdiutil", imageArgs), 5, 1000)
await hdiUtil(imageArgs)
return tempDmg
}

Expand Down
10 changes: 3 additions & 7 deletions packages/dmg-builder/src/dmgUtil.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { exec, retry } from "builder-util"
import { PlatformPackager } from "app-builder-lib"
import { executeFinally } from "builder-util/out/promise"
import * as path from "path"
import { hdiUtil } from "./hdiuil"

export { DmgTarget } from "./dmg"

Expand All @@ -23,7 +23,7 @@ export async function attachAndExecute(dmgPath: string, readWrite: boolean, task
}

args.push(dmgPath)
const attachResult = await exec("hdiutil", args)
const attachResult = await hdiUtil(args)
const deviceResult = attachResult == null ? null : /^(\/dev\/\w+)/.exec(attachResult)
const device = deviceResult == null || deviceResult.length !== 2 ? null : deviceResult[1]
if (device == null) {
Expand All @@ -34,11 +34,7 @@ export async function attachAndExecute(dmgPath: string, readWrite: boolean, task
}

export async function detach(name: string) {
try {
await exec("hdiutil", ["detach", "-quiet", name])
} catch (e: any) {
await retry(() => exec("hdiutil", ["detach", "-force", "-debug", name]), 5, 1000, 500)
}
return hdiUtil(["detach", "-quiet", name])
}

export async function computeBackground(packager: PlatformPackager<any>): Promise<string> {
Expand Down
15 changes: 15 additions & 0 deletions packages/dmg-builder/src/hdiuil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { exec, log, retry } from "builder-util"

export async function hdiUtil(args: string[]) {
return retry(
() => exec("hdiutil", args),
5,
1000,
2000,
0,
(error: any) => {
log.error({ args, code: error.code, error: (error.message || error).toString() }, "unable to execute hdiutil")
return true
}
)
}
Loading