diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 779ba90f0..9c023b117 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -2467,6 +2467,10 @@ "downloadNewVersionMessage": { "message": "Click to download update" }, + "downloadFullNewVersionMessage": { + "message": "Signal couldn’t update. Click to try again.", + "description": "Shown in update dialog when partial update fails and we have to ask user to download full update" + }, "autoUpdateNewVersionInstructions": { "message": "Press Restart Signal to apply the updates." }, diff --git a/ts/components/DialogUpdate.stories.tsx b/ts/components/DialogUpdate.stories.tsx index 455ce9de1..866d01c6d 100644 --- a/ts/components/DialogUpdate.stories.tsx +++ b/ts/components/DialogUpdate.stories.tsx @@ -81,6 +81,18 @@ story.add('Knobs Playground', () => { + + )); + + story.add(`Full Download Ready (${name} container)`, () => ( + + diff --git a/ts/components/DialogUpdate.tsx b/ts/components/DialogUpdate.tsx index 1ef4deb5b..aab7d6d9a 100644 --- a/ts/components/DialogUpdate.tsx +++ b/ts/components/DialogUpdate.tsx @@ -137,6 +137,7 @@ export const DialogUpdate = ({ if ( downloadSize && (dialogType === DialogType.DownloadReady || + dialogType === DialogType.FullDownloadReady || dialogType === DialogType.Downloading) ) { title += ` (${formatFileSize(downloadSize, { round: 0 })})`; @@ -169,8 +170,12 @@ export const DialogUpdate = ({ } let clickLabel: string; + let type: 'warning' | undefined; if (dialogType === DialogType.DownloadReady) { clickLabel = i18n('downloadNewVersionMessage'); + } else if (dialogType === DialogType.FullDownloadReady) { + clickLabel = i18n('downloadFullNewVersionMessage'); + type = 'warning'; } else { clickLabel = i18n('autoUpdateNewVersionMessage'); } @@ -179,6 +184,7 @@ export const DialogUpdate = ({ { ]); }); + it('checks that the data is valid to facilitate caching', async () => { + const oldFilePath = path.join(FIXTURES, oldFile); + const newUrl = `${baseUrl}/${newFile}`; + + const data = await prepareDownload({ + oldFile: oldFilePath, + newUrl, + sha512: newHash, + }); + + assert.isTrue( + isValidPreparedData(data, { + oldFile: oldFilePath, + newUrl, + sha512: newHash, + }) + ); + + assert.isFalse( + isValidPreparedData(data, { + oldFile: 'different file', + newUrl, + sha512: newHash, + }) + ); + + assert.isFalse( + isValidPreparedData(data, { + oldFile: oldFilePath, + newUrl: 'different url', + sha512: newHash, + }) + ); + + assert.isFalse( + isValidPreparedData(data, { + oldFile: oldFilePath, + newUrl, + sha512: 'different hash', + }) + ); + }); + it('downloads the file', async () => { const data = await prepareDownload({ oldFile: path.join(FIXTURES, oldFile), diff --git a/ts/types/Dialogs.ts b/ts/types/Dialogs.ts index c6f90280f..6f359ed8a 100644 --- a/ts/types/Dialogs.ts +++ b/ts/types/Dialogs.ts @@ -9,5 +9,6 @@ export enum DialogType { Cannot_Update = 'Cannot_Update', MacOS_Read_Only = 'MacOS_Read_Only', DownloadReady = 'DownloadReady', + FullDownloadReady = 'FullDownloadReady', Downloading = 'Downloading', } diff --git a/ts/updater/common.ts b/ts/updater/common.ts index 5601c99dd..99912f9df 100644 --- a/ts/updater/common.ts +++ b/ts/updater/common.ts @@ -48,6 +48,7 @@ import { prepareDownload as prepareDifferentialDownload, download as downloadDifferentialData, getBlockMapFileName, + isValidPreparedData as isValidDifferentialData, } from './differential'; const mkdirpPromise = pify(mkdirp); @@ -76,16 +77,35 @@ export type UpdateInformationType = { differentialData: DifferentialDownloadDataType | undefined; }; +enum DownloadMode { + DifferentialOnly = 'DifferentialOnly', + FullOnly = 'FullOnly', + Automatic = 'Automatic', +} + export abstract class Updater { protected fileName: string | undefined; protected version: string | undefined; + protected cachedDifferentialData: DifferentialDownloadDataType | undefined; + + private throttledSendDownloadingUpdate: (downloadedSize: number) => void; + constructor( protected readonly logger: LoggerType, private readonly settingsChannel: SettingsChannel, protected readonly getMainWindow: () => BrowserWindow | undefined - ) {} + ) { + this.throttledSendDownloadingUpdate = throttle((downloadedSize: number) => { + const mainWindow = this.getMainWindow(); + mainWindow?.webContents.send( + 'show-update-dialog', + DialogType.Downloading, + { downloadedSize } + ); + }, 500); + } // // Public APIs @@ -122,9 +142,11 @@ export abstract class Updater { // Protected methods // - protected setUpdateListener(performUpdateCallback: () => void): void { - ipcMain.removeAllListeners('start-update'); - ipcMain.once('start-update', performUpdateCallback); + protected setUpdateListener( + performUpdateCallback: () => Promise + ): void { + ipcMain.removeHandler('start-update'); + ipcMain.handleOnce('start-update', performUpdateCallback); } // @@ -133,8 +155,8 @@ export abstract class Updater { private async downloadAndInstall( updateInfo: UpdateInformationType, - updateOnProgress?: boolean - ): Promise { + mode: DownloadMode + ): Promise { const { logger } = this; const { fileName: newFileName, version: newVersion } = updateInfo; @@ -144,18 +166,20 @@ export abstract class Updater { this.version = newVersion; - let updateFilePath: string; + let updateFilePath: string | undefined; try { - updateFilePath = await this.downloadUpdate( - updateInfo, - updateOnProgress - ); + updateFilePath = await this.downloadUpdate(updateInfo, mode); } catch (error) { // Restore state in case of download error this.version = oldVersion; throw error; } + if (!updateFilePath) { + logger.warn('downloadAndInstall: no update was downloaded'); + return false; + } + const publicKey = hexToBinary(config.get('updatesPublicKey')); const verified = await verifySignature( updateFilePath, @@ -183,8 +207,11 @@ export abstract class Updater { 'downloadAndInstall: no mainWindow, cannot show update dialog' ); } + + return true; } catch (error) { logger.error(`downloadAndInstall: ${Errors.toLogFormat(error)}`); + throw error; } } @@ -192,42 +219,80 @@ export abstract class Updater { const { logger } = this; logger.info('checkForUpdatesMaybeInstall: checking for update...'); - const result = await this.checkForUpdates(force); - if (!result) { + const updateInfo = await this.checkForUpdates(force); + if (!updateInfo) { return; } - const { version: newVersion } = result; + const { version: newVersion } = updateInfo; - if (force || !this.version || gt(newVersion, this.version)) { - const autoDownloadUpdates = await this.getAutoDownloadUpdateSetting(); - if (!autoDownloadUpdates) { - this.setUpdateListener(async () => { - logger.info( - 'checkForUpdatesMaybeInstall: have not downloaded update, going to download' - ); - await this.downloadAndInstall(result, true); - }); - const mainWindow = this.getMainWindow(); - - if (mainWindow) { - mainWindow.webContents.send( - 'show-update-dialog', - DialogType.DownloadReady, - { - downloadSize: result.size, - version: result.version, - } - ); - } else { - logger.warn( - 'checkForUpdatesMaybeInstall: no mainWindow, cannot show update dialog' - ); - } - return; - } - await this.downloadAndInstall(result); + if (!force && this.version && !gt(newVersion, this.version)) { + return; } + + const autoDownloadUpdates = await this.getAutoDownloadUpdateSetting(); + if (autoDownloadUpdates) { + await this.downloadAndInstall(updateInfo, DownloadMode.Automatic); + return; + } + + let mode = DownloadMode.FullOnly; + if (updateInfo.differentialData) { + mode = DownloadMode.DifferentialOnly; + } + + await this.offerUpdate(updateInfo, mode, 0); + } + + private async offerUpdate( + updateInfo: UpdateInformationType, + mode: DownloadMode, + attempt: number + ): Promise { + const { logger } = this; + + this.setUpdateListener(async () => { + logger.info('offerUpdate: have not downloaded update, going to download'); + + const didDownload = await this.downloadAndInstall(updateInfo, mode); + if (!didDownload && mode === DownloadMode.DifferentialOnly) { + this.logger.warn( + 'offerUpdate: Failed to download differential update, offering full' + ); + + return this.offerUpdate(updateInfo, DownloadMode.FullOnly, attempt + 1); + } + + strictAssert(didDownload, 'FullOnly must always download update'); + }); + + const mainWindow = this.getMainWindow(); + if (!mainWindow) { + logger.warn('offerUpdate: no mainWindow, cannot show update dialog'); + return; + } + + let downloadSize: number; + if (mode === DownloadMode.DifferentialOnly) { + strictAssert( + updateInfo.differentialData, + 'Must have differential data in DifferentialOnly mode' + ); + downloadSize = updateInfo.differentialData.downloadSize; + } else { + downloadSize = updateInfo.size; + } + + logger.info(`offerUpdate: offering ${mode} update`); + mainWindow.webContents.send( + 'show-update-dialog', + attempt === 0 ? DialogType.DownloadReady : DialogType.FullDownloadReady, + { + downloadSize, + downloadMode: mode, + version: updateInfo.version, + } + ); } private async checkForUpdates( @@ -278,22 +343,36 @@ export abstract class Updater { `checkForUpdates: Found local installer ${latestInstaller}` ); - try { - differentialData = await prepareDifferentialDownload({ - oldFile: latestInstaller, - newUrl: `${getUpdatesBase()}/${fileName}`, - sha512, - }); + const diffOptions = { + oldFile: latestInstaller, + newUrl: `${getUpdatesBase()}/${fileName}`, + sha512, + }; - this.logger.info( - 'checkForUpdates: differential download size', - differentialData.downloadSize - ); - } catch (error) { - this.logger.error( - 'checkForUpdates: Failed to prepare differential update', - Errors.toLogFormat(error) - ); + if ( + this.cachedDifferentialData && + isValidDifferentialData(this.cachedDifferentialData, diffOptions) + ) { + this.logger.info('checkForUpdates: using cached differential data'); + + differentialData = this.cachedDifferentialData; + } else { + try { + differentialData = await prepareDifferentialDownload(diffOptions); + + this.cachedDifferentialData = differentialData; + + this.logger.info( + 'checkForUpdates: differential download size', + differentialData.downloadSize + ); + } catch (error) { + this.logger.error( + 'checkForUpdates: Failed to prepare differential update', + Errors.toLogFormat(error) + ); + this.cachedDifferentialData = undefined; + } } } @@ -319,11 +398,13 @@ export abstract class Updater { private async downloadUpdate( { fileName, sha512, differentialData }: UpdateInformationType, - updateOnProgress?: boolean - ): Promise { + mode: DownloadMode + ): Promise { const baseUrl = getUpdatesBase(); const updateFileUrl = `${baseUrl}/${fileName}`; + const updateOnProgress = mode !== DownloadMode.Automatic; + const signatureFileName = getSignatureFileName(fileName); const blockMapFileName = getBlockMapFileName(fileName); const signatureUrl = `${baseUrl}/${signatureFileName}`; @@ -356,15 +437,22 @@ export abstract class Updater { const signature = await got(signatureUrl, getGotOptions()).buffer(); await writeFile(targetSignaturePath, signature); - try { - this.logger.info(`downloadUpdate: Downloading blockmap ${blockMapUrl}`); - const blockMap = await got(blockMapUrl, getGotOptions()).buffer(); - await writeFile(targetBlockMapPath, blockMap); - } catch (error) { - this.logger.warn( - 'downloadUpdate: Failed to download blockmap, continuing', - Errors.toLogFormat(error) - ); + if (differentialData) { + this.logger.info(`downloadUpdate: Saving blockmap ${blockMapUrl}`); + await writeFile(targetBlockMapPath, differentialData.newBlockMap); + } else { + try { + this.logger.info( + `downloadUpdate: Downloading blockmap ${blockMapUrl}` + ); + const blockMap = await got(blockMapUrl, getGotOptions()).buffer(); + await writeFile(targetBlockMapPath, blockMap); + } catch (error) { + this.logger.warn( + 'downloadUpdate: Failed to download blockmap, continuing', + Errors.toLogFormat(error) + ); + } } let gotUpdate = false; @@ -384,26 +472,18 @@ export abstract class Updater { } } - if (!gotUpdate && differentialData) { + const isDifferentialEnabled = + differentialData && mode !== DownloadMode.FullOnly; + if (!gotUpdate && isDifferentialEnabled) { this.logger.info( `downloadUpdate: Downloading differential update ${updateFileUrl}` ); try { - const mainWindow = this.getMainWindow(); - - const throttledSend = throttle((downloadedSize: number) => { - mainWindow?.webContents.send( - 'show-update-dialog', - DialogType.Downloading, - { downloadedSize } - ); - }, 500); - await downloadDifferentialData( targetUpdatePath, differentialData, - updateOnProgress ? throttledSend : undefined + updateOnProgress ? this.throttledSendDownloadingUpdate : undefined ); gotUpdate = true; @@ -415,7 +495,8 @@ export abstract class Updater { } } - if (!gotUpdate) { + const isFullEnabled = mode !== DownloadMode.DifferentialOnly; + if (!gotUpdate && isFullEnabled) { this.logger.info( `downloadUpdate: Downloading full update ${updateFileUrl}` ); @@ -426,7 +507,14 @@ export abstract class Updater { ); gotUpdate = true; } - strictAssert(gotUpdate, 'We should get the update one way or another'); + + if (!gotUpdate) { + strictAssert( + mode !== DownloadMode.Automatic && mode !== DownloadMode.FullOnly, + 'Automatic and full mode downloads are guaranteed to happen or error' + ); + return undefined; + } // Now that we successfully downloaded an update - remove old files await Promise.all(oldFiles.map(path => rimrafPromise(path))); @@ -451,21 +539,12 @@ export abstract class Updater { const writeStream = createWriteStream(targetUpdatePath); await new Promise((resolve, reject) => { - const mainWindow = this.getMainWindow(); - if (updateOnProgress && mainWindow) { + if (updateOnProgress) { let downloadedSize = 0; - const throttledSend = throttle(() => { - mainWindow.webContents.send( - 'show-update-dialog', - DialogType.Downloading, - { downloadedSize } - ); - }, 500); - downloadStream.on('data', data => { downloadedSize += data.length; - throttledSend(); + this.throttledSendDownloadingUpdate(downloadedSize); }); } diff --git a/ts/updater/differential.ts b/ts/updater/differential.ts index 55807dc5a..ce7f6d76d 100644 --- a/ts/updater/differential.ts +++ b/ts/updater/differential.ts @@ -53,6 +53,9 @@ export type PrepareDownloadResultType = Readonly<{ newUrl: string; sha512: string; diff: ComputeDiffResultType; + + // This could be used by caller to avoid extra download of the blockmap + newBlockMap: Buffer; }>; export type PrepareDownloadOptionsType = Readonly<{ @@ -182,9 +185,12 @@ export async function prepareDownload({ await readFile(getBlockMapFileName(oldFile)) ); - const newBlockMap = await parseBlockMap( - await got(getBlockMapFileName(newUrl), getGotOptions()).buffer() - ); + const newBlockMapData = await got( + getBlockMapFileName(newUrl), + getGotOptions() + ).buffer(); + + const newBlockMap = await parseBlockMap(newBlockMapData); const diff = computeDiff(oldBlockMap, newBlockMap); @@ -200,10 +206,22 @@ export async function prepareDownload({ diff, oldFile, newUrl, + newBlockMap: newBlockMapData, sha512, }; } +export function isValidPreparedData( + { oldFile, newUrl, sha512 }: PrepareDownloadResultType, + options: PrepareDownloadOptionsType +): boolean { + return ( + oldFile === options.oldFile && + newUrl === options.newUrl && + sha512 === options.sha512 + ); +} + export async function download( newFile: string, { diff, oldFile, newUrl, sha512 }: PrepareDownloadResultType, @@ -221,66 +239,71 @@ export async function download( const gotOptions = getGotOptions(); let downloadedSize = 0; + let isAborted = false; - await pMap( - diff, - async ({ action, readOffset, size, writeOffset }) => { - if (action === 'copy') { - const chunk = Buffer.alloc(size); - const { bytesRead } = await input.read( - chunk, - 0, - chunk.length, - readOffset - ); + try { + await pMap( + diff, + async ({ action, readOffset, size, writeOffset }) => { + if (action === 'copy') { + const chunk = Buffer.alloc(size); + const { bytesRead } = await input.read( + chunk, + 0, + chunk.length, + readOffset + ); - strictAssert( - bytesRead === size, - `Not enough data to read from offset=${readOffset} size=${size}` - ); + strictAssert( + bytesRead === size, + `Not enough data to read from offset=${readOffset} size=${size}` + ); - await output.write(chunk, 0, chunk.length, writeOffset); - - downloadedSize += chunk.length; - statusCallback?.(downloadedSize); - return; - } - - strictAssert(action === 'download', 'invalid action type'); - const stream = got.stream(`${newUrl}`, { - ...gotOptions, - headers: { - range: `bytes=${readOffset}-${readOffset + size - 1}`, - }, - }); - - stream.once('response', ({ statusCode }) => { - if (statusCode !== 206) { - stream.destroy(new Error(`Invalid status code: ${statusCode}`)); + await output.write(chunk, 0, chunk.length, writeOffset); + return; } - }); - let lastOffset = writeOffset; - for await (const chunk of stream) { + strictAssert(action === 'download', 'invalid action type'); + const stream = got.stream(`${newUrl}`, { + ...gotOptions, + headers: { + range: `bytes=${readOffset}-${readOffset + size - 1}`, + }, + }); + + stream.once('response', ({ statusCode }) => { + if (statusCode !== 206) { + stream.destroy(new Error(`Invalid status code: ${statusCode}`)); + } + }); + + let lastOffset = writeOffset; + for await (const chunk of stream) { + strictAssert( + lastOffset - writeOffset + chunk.length <= size, + 'Server returned more data than expected' + ); + await output.write(chunk, 0, chunk.length, lastOffset); + lastOffset += chunk.length; + + downloadedSize += chunk.length; + if (!isAborted) { + statusCallback?.(downloadedSize); + } + } strictAssert( - lastOffset - writeOffset + chunk.length <= size, - 'Server returned more data than expected' + lastOffset - writeOffset === size, + `Not enough data to download from offset=${readOffset} size=${size}` ); - await output.write(chunk, 0, chunk.length, lastOffset); - lastOffset += chunk.length; - - downloadedSize += chunk.length; - statusCallback?.(downloadedSize); - } - strictAssert( - lastOffset - writeOffset === size, - `Not enough data to download from offset=${readOffset} size=${size}` - ); - }, - { concurrency: MAX_CONCURRENCY } - ); - - await Promise.all([input.close(), output.close()]); + }, + { concurrency: MAX_CONCURRENCY } + ); + } catch (error) { + isAborted = true; + throw error; + } finally { + await Promise.all([input.close(), output.close()]); + } const checkResult = await checkIntegrity(tempFile, sha512); strictAssert(checkResult.ok, checkResult.error ?? ''); diff --git a/ts/updater/macos.ts b/ts/updater/macos.ts index d4ea58fa5..678ea2286 100644 --- a/ts/updater/macos.ts +++ b/ts/updater/macos.ts @@ -57,7 +57,7 @@ export class MacOSUpdater extends Updater { // because Squirrel has cached the update file and will do the right thing. logger.info('downloadAndInstall: showing update dialog...'); - this.setUpdateListener(() => { + this.setUpdateListener(async () => { logger.info('performUpdate: calling quitAndInstall...'); markShouldQuit(); autoUpdater.quitAndInstall();