From f966f1a22bd7577e493595ac261e4b38ec0ce233 Mon Sep 17 00:00:00 2001 From: develar Date: Thu, 26 Jul 2018 20:06:16 +0200 Subject: [PATCH] feat(electron-updater): download update on macOS in the same way as for other OS Much more reliable to download file and only then pipe it to Squirrel.Mac then proxy directly It in any case required for upcoming delta updates. Close #3168 --- .../electron-updater/src/AppImageUpdater.ts | 16 +- packages/electron-updater/src/AppUpdater.ts | 116 ++++++++++++++- packages/electron-updater/src/BaseUpdater.ts | 118 ++------------- packages/electron-updater/src/MacUpdater.ts | 140 ++++++++---------- packages/electron-updater/src/NsisUpdater.ts | 15 +- .../__snapshots__/macUpdaterTest.js.snap | 1 + test/src/updater/macUpdaterTest.ts | 5 +- 7 files changed, 202 insertions(+), 209 deletions(-) diff --git a/packages/electron-updater/src/AppImageUpdater.ts b/packages/electron-updater/src/AppImageUpdater.ts index 47636541284..fcc3c0ec4ea 100644 --- a/packages/electron-updater/src/AppImageUpdater.ts +++ b/packages/electron-updater/src/AppImageUpdater.ts @@ -1,4 +1,4 @@ -import { AllPublishOptions, DownloadOptions, newError } from "builder-util-runtime" +import { AllPublishOptions, newError } from "builder-util-runtime" import { execFileSync, spawn } from "child_process" import isDev from "electron-is-dev" import { chmod, unlinkSync } from "fs-extra-p" @@ -37,21 +37,11 @@ export class AppImageUpdater extends BaseUpdater { protected async doDownloadUpdate(downloadUpdateOptions: DownloadUpdateOptions): Promise> { const provider = await this.provider const fileInfo = findFile(provider.resolveFiles(downloadUpdateOptions.updateInfo), "AppImage")!! - - const downloadOptions: DownloadOptions = { - skipDirCreation: true, - headers: downloadUpdateOptions.requestHeaders, - cancellationToken: downloadUpdateOptions.cancellationToken, - sha2: (fileInfo.info as any).sha2, - sha512: fileInfo.info.sha512, - } - return await this.executeDownload({ fileExtension: "AppImage", - downloadOptions, fileInfo, - updateInfo: downloadUpdateOptions.updateInfo, - task: async updateFile => { + downloadUpdateOptions, + task: async (updateFile, downloadOptions) => { const oldFile = process.env.APPIMAGE!! if (oldFile == null) { throw newError("APPIMAGE env is not defined", "ERR_UPDATER_OLD_FILE_NOT_FOUND") diff --git a/packages/electron-updater/src/AppUpdater.ts b/packages/electron-updater/src/AppUpdater.ts index a755413a48a..51b7c01c87a 100644 --- a/packages/electron-updater/src/AppUpdater.ts +++ b/packages/electron-updater/src/AppUpdater.ts @@ -1,9 +1,9 @@ -import { AllPublishOptions, asArray, CancellationToken, newError, PublishConfiguration, UpdateInfo, UUID } from "builder-util-runtime" +import { AllPublishOptions, asArray, CancellationToken, newError, PublishConfiguration, UpdateInfo, UUID, DownloadOptions, CancellationError } from "builder-util-runtime" import { randomBytes } from "crypto" import { Notification } from "electron" import isDev from "electron-is-dev" import { EventEmitter } from "events" -import { outputFile, readFile } from "fs-extra-p" +import { ensureDir, outputFile, readFile, rename, unlink } from "fs-extra-p" import { OutgoingHttpHeaders } from "http" import { safeLoad } from "js-yaml" import { Lazy } from "lazy-val" @@ -13,7 +13,7 @@ import "source-map-support/register" import { DownloadedUpdateHelper } from "./DownloadedUpdateHelper" import { ElectronHttpExecutor } from "./electronHttpExecutor" import { GenericProvider } from "./providers/GenericProvider" -import { Logger, Provider, UpdateCheckResult, UpdaterSignal } from "./main" +import { DOWNLOAD_PROGRESS, Logger, Provider, ResolvedUpdateFileInfo, UPDATE_DOWNLOADED, UpdateCheckResult, UpdaterSignal } from "./main" import { createClient, isUrlProbablySupportMultiRangeRequests } from "./providerFactory" export abstract class AppUpdater extends EventEmitter { @@ -483,6 +483,107 @@ export abstract class AppUpdater extends EventEmitter { } return true } + + protected async executeDownload(taskOptions: DownloadExecutorTask): Promise> { + const fileInfo = taskOptions.fileInfo + const downloadOptions: DownloadOptions = { + skipDirCreation: true, + headers: taskOptions.downloadUpdateOptions.requestHeaders, + cancellationToken: taskOptions.downloadUpdateOptions.cancellationToken, + sha2: (fileInfo.info as any).sha2, + sha512: fileInfo.info.sha512, + } + + if (this.listenerCount(DOWNLOAD_PROGRESS) > 0) { + downloadOptions.onProgress = it => this.emit(DOWNLOAD_PROGRESS, it) + } + + const updateInfo = taskOptions.downloadUpdateOptions.updateInfo + const version = updateInfo.version + const packageInfo = fileInfo.packageInfo + + function getCacheUpdateFileName(): string { + // bloody NodeJS URL doesn't decode automatically + const urlPath = decodeURIComponent(taskOptions.fileInfo.url.pathname) + if (urlPath.endsWith(`.${taskOptions.fileExtension}`)) { + return path.posix.basename(urlPath) + } + else { + // url like /latest, generate name + return `update.${taskOptions.fileExtension}` + } + } + + const cacheDir = this.downloadedUpdateHelper.cacheDir + await ensureDir(cacheDir) + const updateFileName = getCacheUpdateFileName() + let updateFile = path.join(cacheDir, updateFileName) + const packageFile = packageInfo == null ? null : path.join(cacheDir, `package-${version}${path.extname(packageInfo.path) || ".7z"}`) + + const done = async (isSaveCache: boolean) => { + this.downloadedUpdateHelper.setDownloadedFile(updateFile, packageFile, updateInfo, fileInfo) + if (isSaveCache) { + await this.downloadedUpdateHelper.cacheUpdateInfo(updateFileName) + } + + await taskOptions.done!!(updateFile) + + this.emit(UPDATE_DOWNLOADED, updateInfo) + return packageFile == null ? [updateFile] : [updateFile, packageFile] + } + + const log = this._logger + const cachedUpdateFile = await this.downloadedUpdateHelper.validateDownloadedPath(updateFile, updateInfo, fileInfo, log) + if (cachedUpdateFile != null) { + updateFile = cachedUpdateFile + return await done(false) + } + + const removeFileIfAny = async () => { + await this.downloadedUpdateHelper.clear() + .catch(() => { + // ignore + }) + return await unlink(updateFile) + .catch(() => { + // ignore + }) + } + + // https://github.com/electron-userland/electron-builder/pull/2474#issuecomment-366481912 + let nameCounter = 0 + let tempUpdateFile = path.join(cacheDir, `temp-${updateFileName}`) + for (let i = 0; i < 3; i++) { + try { + await unlink(tempUpdateFile) + } + catch (e) { + if (e.code === "ENOENT") { + break + } + + log.warn(`Error on remove temp update file: ${e}`) + tempUpdateFile = path.join(cacheDir, `temp-${nameCounter++}-${updateFileName}`) + } + } + + try { + await taskOptions.task(tempUpdateFile, downloadOptions, packageFile, removeFileIfAny) + await rename(tempUpdateFile, updateFile) + } + catch (e) { + await removeFileIfAny() + + if (e instanceof CancellationError) { + log.info("Cancelled") + this.emit("update-cancelled", updateInfo) + } + throw e + } + + log.info(`New version ${version} has been downloaded to ${updateFile}`) + return await done(true) + } } export interface DownloadUpdateOptions { @@ -510,3 +611,12 @@ export class NoOpLogger implements Logger { // ignore } } + +export interface DownloadExecutorTask { + readonly fileExtension: string + readonly fileInfo: ResolvedUpdateFileInfo + readonly downloadUpdateOptions: DownloadUpdateOptions + readonly task: (destinationFile: string, downloadOptions: DownloadOptions, packageFile: string | null, removeTempDirIfAny: () => Promise) => Promise + + readonly done?: (destinationFile: string) => Promise +} \ No newline at end of file diff --git a/packages/electron-updater/src/BaseUpdater.ts b/packages/electron-updater/src/BaseUpdater.ts index e5819553615..96a33493ab2 100644 --- a/packages/electron-updater/src/BaseUpdater.ts +++ b/packages/electron-updater/src/BaseUpdater.ts @@ -1,8 +1,5 @@ -import { AllPublishOptions, CancellationError, DownloadOptions, UpdateInfo } from "builder-util-runtime" -import { ensureDir, rename, unlink } from "fs-extra-p" -import * as path from "path" -import { AppUpdater } from "./AppUpdater" -import { DOWNLOAD_PROGRESS, ResolvedUpdateFileInfo, UPDATE_DOWNLOADED } from "./main" +import { AllPublishOptions } from "builder-util-runtime" +import { AppUpdater, DownloadExecutorTask } from "./AppUpdater" export abstract class BaseUpdater extends AppUpdater { protected quitAndInstallCalled = false @@ -21,101 +18,19 @@ export abstract class BaseUpdater extends AppUpdater { this.app.quit() } }) - } else { + } + else { this.quitAndInstallCalled = false } } - protected async executeDownload(taskOptions: DownloadExecutorTask): Promise> { - if (this.listenerCount(DOWNLOAD_PROGRESS) > 0) { - taskOptions.downloadOptions.onProgress = it => this.emit(DOWNLOAD_PROGRESS, it) - } - - const updateInfo = taskOptions.updateInfo - const version = updateInfo.version - const fileInfo = taskOptions.fileInfo - const packageInfo = fileInfo.packageInfo - - function getCacheUpdateFileName(): string { - // bloody NodeJS URL doesn't decode automatically - const urlPath = decodeURIComponent(taskOptions.fileInfo.url.pathname) - if (urlPath.endsWith(`.${taskOptions.fileExtension}`)) { - return path.posix.basename(urlPath) - } - else { - // url like /latest, generate name - return `update.${taskOptions.fileExtension}` - } - } - - const cacheDir = this.downloadedUpdateHelper.cacheDir - await ensureDir(cacheDir) - const updateFileName = getCacheUpdateFileName() - let updateFile = path.join(cacheDir, updateFileName) - const packageFile = packageInfo == null ? null : path.join(cacheDir, `package-${version}${path.extname(packageInfo.path) || ".7z"}`) - - const done = async (isSaveCache: boolean) => { - this.downloadedUpdateHelper.setDownloadedFile(updateFile, packageFile, updateInfo, fileInfo) - if (isSaveCache) { - await this.downloadedUpdateHelper.cacheUpdateInfo(updateFileName) - } - - this.addQuitHandler() - this.emit(UPDATE_DOWNLOADED, updateInfo) - return packageFile == null ? [updateFile] : [updateFile, packageFile] - } - - const log = this._logger - const cachedUpdateFile = await this.downloadedUpdateHelper.validateDownloadedPath(updateFile, updateInfo, fileInfo, log) - if (cachedUpdateFile != null) { - updateFile = cachedUpdateFile - return await done(false) - } - - const removeFileIfAny = async () => { - await this.downloadedUpdateHelper.clear() - .catch(() => { - // ignore - }) - return await unlink(updateFile) - .catch(() => { - // ignore - }) - } - - // https://github.com/electron-userland/electron-builder/pull/2474#issuecomment-366481912 - let nameCounter = 0 - let tempUpdateFile = path.join(cacheDir, `temp-${updateFileName}`) - for (let i = 0; i < 3; i++) { - try { - await unlink(tempUpdateFile) + protected executeDownload(taskOptions: DownloadExecutorTask): Promise> { + return super.executeDownload({ + ...taskOptions, + done: async () => { + this.addQuitHandler() } - catch (e) { - if (e.code === "ENOENT") { - break - } - - log.warn(`Error on remove temp update file: ${e}`) - tempUpdateFile = path.join(cacheDir, `temp-${nameCounter++}-${updateFileName}`) - } - } - - try { - await taskOptions.task(tempUpdateFile, packageFile, removeFileIfAny) - await rename(tempUpdateFile, updateFile) - } - catch (e) { - await removeFileIfAny() - - if (e instanceof CancellationError) { - log.info("Cancelled") - this.emit("update-cancelled", updateInfo) - } - throw e - } - - log.info(`New version ${version} has been downloaded to ${updateFile}`) - return await done(true) + }) } protected abstract doInstall(installerPath: string, isSilent: boolean, isRunAfter: boolean): boolean @@ -158,17 +73,10 @@ export abstract class BaseUpdater extends AppUpdater { if (!this.quitAndInstallCalled) { this._logger.info("Auto install update on quit") await this.install(true, false) - } else { + } + else { this._logger.info("Update installer has already been triggered. Quitting application.") } }) } -} - -export interface DownloadExecutorTask { - readonly fileExtension: string - readonly downloadOptions: DownloadOptions - readonly fileInfo: ResolvedUpdateFileInfo - readonly updateInfo: UpdateInfo - readonly task: (destinationFile: string, packageFile: string | null, removeTempDirIfAny: () => Promise) => Promise -} +} \ No newline at end of file diff --git a/packages/electron-updater/src/MacUpdater.ts b/packages/electron-updater/src/MacUpdater.ts index c62e5db80bb..0e6bcb0e672 100644 --- a/packages/electron-updater/src/MacUpdater.ts +++ b/packages/electron-updater/src/MacUpdater.ts @@ -1,9 +1,10 @@ -import { AllPublishOptions, CancellationToken, configureRequestOptionsFromUrl, DigestTransform, newError, ProgressCallbackTransform, RequestHeaders, safeGetHeader, safeStringifyJson } from "builder-util-runtime" +import { AllPublishOptions, CancellationToken, configureRequestOptionsFromUrl, DigestTransform, newError, RequestHeaders, safeStringifyJson } from "builder-util-runtime" import { createServer, IncomingMessage, OutgoingHttpHeaders, ServerResponse } from "http" import { AddressInfo } from "net" import { AppUpdater, DownloadUpdateOptions } from "./AppUpdater" import { DOWNLOAD_PROGRESS, UPDATE_DOWNLOADED } from "./main" import { findFile } from "./providers/Provider" +import { createReadStream, stat } from "fs-extra-p" import AutoUpdater = Electron.AutoUpdater export class MacUpdater extends AppUpdater { @@ -39,96 +40,85 @@ export class MacUpdater extends AppUpdater { return `http://${address.address}:${address.port}` } - return await new Promise>((resolve, reject) => { - server.on("request", (request: IncomingMessage, response: ServerResponse) => { - const requestUrl = request.url! - this._logger.info(`${requestUrl} requested`) - if (requestUrl === "/") { - const data = Buffer.from(`{ "url": "${getServerUrl()}/app.zip" }`) - response.writeHead(200, {"Content-Type": "application/json", "Content-Length": data.length}) - response.end(data) + return await this.executeDownload({ + fileExtension: "zip", + fileInfo: zipFileInfo, + downloadUpdateOptions, + task: (destinationFile, downloadOptions) => { + return this.httpExecutor.download(zipFileInfo.url.href, destinationFile, downloadOptions) + }, + done: async updateFile => { + let updateFileSize = zipFileInfo.info.size + if (updateFileSize == null) { + updateFileSize = (await stat(updateFile)).size } - else if (requestUrl.startsWith("/app.zip")) { - const debug = this._logger.debug - let errorOccurred = false - response.on("finish", () => { - try { - setImmediate(() => server.close()) + + return await new Promise>((resolve, reject) => { + server.on("request", (request: IncomingMessage, response: ServerResponse) => { + const requestUrl = request.url!! + this._logger.info(`${requestUrl} requested`) + if (requestUrl === "/") { + const data = Buffer.from(`{ "url": "${getServerUrl()}/app.zip" }`) + response.writeHead(200, {"Content-Type": "application/json", "Content-Length": data.length}) + response.end(data) } - finally { - if (!errorOccurred) { - this.nativeUpdater.removeListener("error", reject) - resolve([]) - } + else if (requestUrl.startsWith("/app.zip")) { + let errorOccurred = false + response.on("finish", () => { + try { + setImmediate(() => server.close()) + } + finally { + if (!errorOccurred) { + this.nativeUpdater.removeListener("error", reject) + resolve([]) + } + } + }) + + this._logger.info(`app.zip requested by Squirrel.Mac, pipe ${updateFile}`) + + const readStream = createReadStream(updateFile) + readStream.on("error", error => { + try { + response.end() + } + catch (e) { + errorOccurred = true + this.nativeUpdater.removeListener("error", reject) + reject(new Error(`Cannot pipe "${updateFile}": ${error}`)) + } + }) + + response.writeHead(200, { + "Content-Type": "application/zip", + "Content-Length": updateFileSize, + }) + readStream.pipe(response) } - }) - - if (debug != null) { - debug(`app.zip requested by Squirrel.Mac, download ${zipFileInfo.url.href}`) - } - this.doProxyUpdateFile(response, zipFileInfo.url.href, downloadUpdateOptions.requestHeaders, zipFileInfo.info.sha512, downloadUpdateOptions.cancellationToken, error => { - errorOccurred = true - try { - response.writeHead(500) + else { + this._logger.warn(`${requestUrl} requested, but not supported`) + response.writeHead(404) response.end() } - finally { - this.nativeUpdater.removeListener("error", reject) - reject(new Error(`Cannot download "${zipFileInfo.url}": ${error}`)) - } }) - } - else { - this._logger.warn(`${requestUrl} requested, but not supported`) - response.writeHead(404) - response.end() - } - }) - server.listen(0, "127.0.0.1", 16, () => { - this.nativeUpdater.setFeedURL(`${getServerUrl()}`, {"Cache-Control": "no-cache"}) + server.listen(0, "127.0.0.1", 8, () => { + this.nativeUpdater.setFeedURL(`${getServerUrl()}`, {"Cache-Control": "no-cache"}) - this.nativeUpdater.once("error", reject) - this.nativeUpdater.checkForUpdates() - }) + this.nativeUpdater.once("error", reject) + this.nativeUpdater.checkForUpdates() + }) + }) + } }) } private doProxyUpdateFile(nativeResponse: ServerResponse, url: string, headers: OutgoingHttpHeaders, sha512: string | null, cancellationToken: CancellationToken, errorHandler: (error: Error) => void) { const downloadRequest = this.httpExecutor.doRequest(configureRequestOptionsFromUrl(url, {headers}), downloadResponse => { - const statusCode = downloadResponse.statusCode - if (statusCode >= 400) { - this._logger.warn(`Request to ${url} failed, status code: ${statusCode}`) - - try { - nativeResponse.writeHead(statusCode) - nativeResponse.end() - } - finally { - errorHandler(new Error(`Cannot download "${url}", status ${statusCode}: ${downloadResponse.statusMessage}`)) - } - return - } - - // in tests Electron NET Api is not used, so, we have to handle redirect. - const redirectUrl = safeGetHeader(downloadResponse, "location") - if (redirectUrl != null) { - this.doProxyUpdateFile(nativeResponse, redirectUrl, headers, sha512, cancellationToken, errorHandler) - return - } - const nativeHeaders: RequestHeaders = {"Content-Type": "application/zip"} const streams: Array = [] const downloadListenerCount = this.listenerCount(DOWNLOAD_PROGRESS) this._logger.info(`${DOWNLOAD_PROGRESS} listener count: ${downloadListenerCount}`) - if (downloadListenerCount > 0) { - const contentLength = safeGetHeader(downloadResponse, "content-length") - this._logger.info(`contentLength: ${contentLength}`) - if (contentLength != null) { - nativeHeaders["Content-Length"] = contentLength - streams.push(new ProgressCallbackTransform(parseInt(contentLength, 10), cancellationToken, it => this.emit(DOWNLOAD_PROGRESS, it))) - } - } - nativeResponse.writeHead(200, nativeHeaders) // for mac only sha512 is produced (sha256 is published for windows only to preserve backward compatibility) diff --git a/packages/electron-updater/src/NsisUpdater.ts b/packages/electron-updater/src/NsisUpdater.ts index a5ee62ce367..9be254606f4 100644 --- a/packages/electron-updater/src/NsisUpdater.ts +++ b/packages/electron-updater/src/NsisUpdater.ts @@ -1,4 +1,4 @@ -import { AllPublishOptions, DownloadOptions, newError, PackageFileInfo, BlockMap, CURRENT_APP_PACKAGE_FILE_NAME, CURRENT_APP_INSTALLER_FILE_NAME } from "builder-util-runtime" +import { AllPublishOptions, newError, PackageFileInfo, BlockMap, CURRENT_APP_PACKAGE_FILE_NAME, CURRENT_APP_INSTALLER_FILE_NAME } from "builder-util-runtime" import { spawn } from "child_process" import { OutgoingHttpHeaders } from "http" import * as path from "path" @@ -22,20 +22,11 @@ export class NsisUpdater extends BaseUpdater { protected async doDownloadUpdate(downloadUpdateOptions: DownloadUpdateOptions): Promise> { const provider = await this.provider const fileInfo = findFile(provider.resolveFiles(downloadUpdateOptions.updateInfo), "exe")!! - const downloadOptions: DownloadOptions = { - skipDirCreation: true, - headers: downloadUpdateOptions.requestHeaders, - cancellationToken: downloadUpdateOptions.cancellationToken, - sha2: (fileInfo.info as any).sha2, - sha512: fileInfo.info.sha512, - } - return await this.executeDownload({ fileExtension: "exe", - downloadOptions, + downloadUpdateOptions, fileInfo, - updateInfo: downloadUpdateOptions.updateInfo, - task: async (destinationFile, packageFile, removeTempDirIfAny) => { + task: async (destinationFile, downloadOptions, packageFile, removeTempDirIfAny) => { const packageInfo = fileInfo.packageInfo const isWebInstaller = packageInfo != null && packageFile != null if (isWebInstaller || await this.differentialDownloadInstaller(fileInfo, downloadUpdateOptions, destinationFile, downloadUpdateOptions.requestHeaders, provider)) { diff --git a/test/out/updater/__snapshots__/macUpdaterTest.js.snap b/test/out/updater/__snapshots__/macUpdaterTest.js.snap index ef3a0e3c1ed..122e2f6a0f6 100644 --- a/test/out/updater/__snapshots__/macUpdaterTest.js.snap +++ b/test/out/updater/__snapshots__/macUpdaterTest.js.snap @@ -4,5 +4,6 @@ exports[`mac updates 1`] = ` Array [ "checking-for-update", "update-available", + "update-downloaded", ] `; diff --git a/test/src/updater/macUpdaterTest.ts b/test/src/updater/macUpdaterTest.ts index e43ad14da65..58ea9ba2c75 100644 --- a/test/src/updater/macUpdaterTest.ts +++ b/test/src/updater/macUpdaterTest.ts @@ -2,6 +2,7 @@ import { configureRequestOptionsFromUrl, GithubOptions } from "builder-util-runt import { httpExecutor } from "builder-util/out/nodeHttpExecutor" import { MacUpdater } from "electron-updater/out/MacUpdater" import { EventEmitter } from "events" +import { assertThat } from "../helpers/fileAssert" import { createTestApp, trackEvents, tuneNsisUpdater, writeUpdateConfig } from "../helpers/updaterTestUtil" class TestNativeUpdater extends EventEmitter { @@ -57,6 +58,8 @@ test.ifAll.ifNotCi.ifMac("mac updates", async () => { const updateCheckResult = await updater.checkForUpdates() // todo when will be updated to use files // expect(removeUnstableProperties(updateCheckResult.updateInfo.files)).toMatchSnapshot() - expect(await updateCheckResult.downloadPromise).toEqual([]) + const files = await updateCheckResult.downloadPromise + expect(files!!.length).toEqual(1) + await assertThat(files!![0]).isFile() expect(actualEvents).toMatchSnapshot() }) \ No newline at end of file