Display differential download size in UI

_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."
   },

ts/components/DialogUpdate.stories.tsx

@@ -81,6 +81,18 @@ story.add('Knobs Playground', () => {
       <DialogUpdate
         {...defaultPropsForBreakpoint}
         dialogType={DialogType.DownloadReady}
+        downloadSize={30123456}
+        currentVersion="5.24.0"
+      />
+    </FakeLeftPaneContainer>
+  ));
+
+  story.add(`Full Download Ready (${name} container)`, () => (
+    <FakeLeftPaneContainer containerWidthBreakpoint={containerWidthBreakpoint}>
+      <DialogUpdate
+        {...defaultPropsForBreakpoint}
+        dialogType={DialogType.FullDownloadReady}
+        downloadSize={300123456}
         currentVersion="5.24.0"
       />
     </FakeLeftPaneContainer>

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 = ({
     <LeftPaneDialog
       containerWidthBreakpoint={containerWidthBreakpoint}
       icon="update"
+      type={type}
       title={title}
       hoverText={versionTitle}
       hasAction

ts/shims/updateIpc.ts

@@ -4,5 +4,5 @@
 import { ipcRenderer } from 'electron';
 
 export function startUpdate(): void {
-  ipcRenderer.send('start-update');
+  ipcRenderer.invoke('start-update');
 }

ts/test-node/updater/differential_test.ts

@@ -12,6 +12,7 @@ import {
   computeDiff,
   getBlockMapFileName,
   prepareDownload,
+  isValidPreparedData,
   download,
 } from '../../updater/differential';
 
@@ -143,6 +144,49 @@ describe('updater/differential', () => {
       ]);
     });
 
+    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),

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',
 }

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 {
+  protected setUpdateListener(
-    ipcMain.removeAllListeners('start-update');
+    performUpdateCallback: () => Promise<void>
-    ipcMain.once('start-update', performUpdateCallback);
+  ): 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
+    mode: DownloadMode
-  ): Promise<void> {
+  ): Promise<boolean> {
     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(
+        updateFilePath = await this.downloadUpdate(updateInfo, mode);
-          updateInfo,
-          updateOnProgress
-        );
       } 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);
+    const updateInfo = await this.checkForUpdates(force);
-    if (!result) {
+    if (!updateInfo) {
       return;
     }
 
-    const { version: newVersion } = result;
+    const { version: newVersion } = updateInfo;
 
-    if (force || !this.version || gt(newVersion, this.version)) {
+    if (!force && this.version && !gt(newVersion, this.version)) {
-      const autoDownloadUpdates = await this.getAutoDownloadUpdateSetting();
+      return;
-      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);
     }
+
+    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<void> {
+    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 {
+      const diffOptions = {
-        differentialData = await prepareDifferentialDownload({
+        oldFile: latestInstaller,
-          oldFile: latestInstaller,
+        newUrl: `${getUpdatesBase()}/${fileName}`,
-          newUrl: `${getUpdatesBase()}/${fileName}`,
+        sha512,
-          sha512,
+      };
-        });
 
-        this.logger.info(
+      if (
-          'checkForUpdates: differential download size',
+        this.cachedDifferentialData &&
-          differentialData.downloadSize
+        isValidDifferentialData(this.cachedDifferentialData, diffOptions)
-        );
+      ) {
-      } catch (error) {
+        this.logger.info('checkForUpdates: using cached differential data');
-        this.logger.error(
+
-          'checkForUpdates: Failed to prepare differential update',
+        differentialData = this.cachedDifferentialData;
-          Errors.toLogFormat(error)
+      } 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
+    mode: DownloadMode
-  ): Promise<string> {
+  ): Promise<string | undefined> {
     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 {
+      if (differentialData) {
-        this.logger.info(`downloadUpdate: Downloading blockmap ${blockMapUrl}`);
+        this.logger.info(`downloadUpdate: Saving blockmap ${blockMapUrl}`);
-        const blockMap = await got(blockMapUrl, getGotOptions()).buffer();
+        await writeFile(targetBlockMapPath, differentialData.newBlockMap);
-        await writeFile(targetBlockMapPath, blockMap);
+      } else {
-      } catch (error) {
+        try {
-        this.logger.warn(
+          this.logger.info(
-          'downloadUpdate: Failed to download blockmap, continuing',
+            `downloadUpdate: Downloading blockmap ${blockMapUrl}`
-          Errors.toLogFormat(error)
+          );
-        );
+          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<void>((resolve, reject) => {
-      const mainWindow = this.getMainWindow();
+      if (updateOnProgress) {
-      if (updateOnProgress && mainWindow) {
         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);
         });
       }
 

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(
+  const newBlockMapData = await got(
-    await got(getBlockMapFileName(newUrl), getGotOptions()).buffer()
+    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(
+  try {
-    diff,
+    await pMap(
-    async ({ action, readOffset, size, writeOffset }) => {
+      diff,
-      if (action === 'copy') {
+      async ({ action, readOffset, size, writeOffset }) => {
-        const chunk = Buffer.alloc(size);
+        if (action === 'copy') {
-        const { bytesRead } = await input.read(
+          const chunk = Buffer.alloc(size);
-          chunk,
+          const { bytesRead } = await input.read(
-          0,
+            chunk,
-          chunk.length,
+            0,
-          readOffset
+            chunk.length,
-        );
+            readOffset
+          );
 
-        strictAssert(
+          strictAssert(
-          bytesRead === size,
+            bytesRead === size,
-          `Not enough data to read from offset=${readOffset} size=${size}`
+            `Not enough data to read from offset=${readOffset} size=${size}`
-        );
+          );
 
-        await output.write(chunk, 0, chunk.length, writeOffset);
+          await output.write(chunk, 0, chunk.length, writeOffset);
-
+          return;
-        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}`));
         }
-      });
 
-      let lastOffset = writeOffset;
+        strictAssert(action === 'download', 'invalid action type');
-      for await (const chunk of stream) {
+        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,
+          lastOffset - writeOffset === size,
-          'Server returned more data than expected'
+          `Not enough data to download from offset=${readOffset} size=${size}`
         );
-        await output.write(chunk, 0, chunk.length, lastOffset);
+      },
-        lastOffset += chunk.length;
+      { concurrency: MAX_CONCURRENCY }
-
+    );
-        downloadedSize += chunk.length;
+  } catch (error) {
-        statusCallback?.(downloadedSize);
+    isAborted = true;
-      }
+    throw error;
-      strictAssert(
+  } finally {
-        lastOffset - writeOffset === size,
+    await Promise.all([input.close(), output.close()]);
-        `Not enough data to download from offset=${readOffset} size=${size}`
+  }
-      );
-    },
-    { concurrency: MAX_CONCURRENCY }
-  );
-
-  await Promise.all([input.close(), output.close()]);
 
   const checkResult = await checkIntegrity(tempFile, sha512);
   strictAssert(checkResult.ok, checkResult.error ?? '');

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();