Fix DownloadItem not being properly initialized (#6)

CHANGELOG.md

@@ -1,3 +1,17 @@
+# 3.0.0 (2024-04-04)
+
+Adds fixes around `DownloadData` population.
+
+**Breaking changes**
+
+`DownloadManager.download()` is now `async`.
+
+This change is necessary to fix a race condition where `download()` is called, but if you immediately try to perform an
+operation against the returned id, such as `pauseDownload()`, the `DownloadItem` has not been properly initialized
+since the event that populates `DownloadItem` is out-of-band.
+
+So the new `download()` implementation waits until `DownloadItem` is properly initialized before returning the id.
+
 # 2.4.1 (2024-04-03)
 
 - Fix issue where pausing a download won't pause it

README.md

@@ -25,7 +25,7 @@ import { ElectronDownloadManager } from 'electron-dl-manager';
 const manager = new ElectronDownloadManager();
 
 // Start a download
-manager.download({
+const id = await manager.download({
   window: browserWindowInstance,
   url: 'https://example.com/file.zip',
   saveDialogOptions: {
@@ -106,7 +106,7 @@ ipcMain.handle('download-file', async (event, args) => {
   let downloadId
   const browserWindow = BrowserWindow.fromId(event.sender.id)
 
-  downloadId = manager.download({
+  downloadId = await manager.download({
     window: browserWindow,
     url,
     // If you want to download without a save as dialog
@@ -183,7 +183,7 @@ interface DownloadManagerConstructorParams {
 Starts a file download. Returns the `id` of the download.
 
 ```typescript
-download(params: DownloadParams): string
+download(params: DownloadParams): Promise<string>
 ```
 
 #### Interface: `DownloadParams`

biome.json

@@ -11,6 +11,9 @@
   "linter": {
     "enabled": true,
     "rules": {
+      "complexity": {
+        "useLiteralKeys": "off"
+      },
       "suspicious": {
         "noImplicitAnyLet": "off",
         "noExplicitAny": "off"

package.json

@@ -1,6 +1,6 @@
 {
   "name": "electron-dl-manager",
-  "version": "2.4.1",
+  "version": "3.0.0",
   "description": "A library for implementing file downloads in Electron with 'save as' dialog and id support.",
   "main": "dist/cjs/index.js",
   "module": "dist/esm/index.js",

src/DownloadInitiator.ts

@@ -8,6 +8,7 @@ import { calculateDownloadMetrics, determineFilePath } from "./utils";
 interface DownloadInitiatorConstructorParams {
   debugLogger?: (message: string) => void;
   onCleanup?: (id: DownloadData) => void;
+  onDownloadInit?: (id: DownloadData) => void;
 }
 
 interface WillOnDownloadParams {
@@ -51,6 +52,10 @@ export class DownloadInitiator {
    * The handler for the DownloadItem's `done` event.
    */
   private onItemDone: (event: Event, state: "completed" | "cancelled" | "interrupted") => Promise<void>;
+  /**
+   * When the download is initiated
+   */
+  private onDownloadInit: (data: DownloadData) => void;
   /**
    * When cleanup is called
    */
@@ -71,6 +76,7 @@ export class DownloadInitiator {
     this.onItemUpdated = () => Promise.resolve();
     this.onItemDone = () => Promise.resolve();
     this.onCleanup = config.onCleanup || (() => {});
+    this.onDownloadInit = config.onDownloadInit || (() => {});
     this.config = {} as DownloadConfig;
     this.callbackDispatcher = {} as CallbackDispatcher;
   }
@@ -107,6 +113,10 @@ export class DownloadInitiator {
       this.downloadData.webContents = webContents;
       this.downloadData.event = event;
 
+      if (this.onDownloadInit) {
+        this.onDownloadInit(this.downloadData);
+      }
+
       if (this.config.saveDialogOptions) {
         this.initSaveAsInteractiveDownload();
         return;
@@ -143,7 +153,7 @@ export class DownloadInitiator {
       if (item.getSavePath()) {
         clearInterval(interval);
 
-        this.log(`User selected save path to ${this.downloadData.item.getSavePath()}`);
+        this.log(`User selected save path to ${item.getSavePath()}`);
         this.log("Initiating download item handlers");
 
         this.downloadData.resolvedFilename = path.basename(item.getSavePath());
@@ -159,7 +169,7 @@ export class DownloadInitiator {
           item.once("done", this.generateItemOnDone());
         }
 
-        if (!item['_userInitiatedPause']) {
+        if (!item["_userInitiatedPause"]) {
           item.resume();
         }
       } else if (this.downloadData.isDownloadCancelled()) {
@@ -176,11 +186,11 @@ export class DownloadInitiator {
   private augmentDownloadItem(item: DownloadItem) {
     // This covers if the user manually pauses the download
     // before we have set up the event listeners on the item
-    item['_userInitiatedPause'] = false;
+    item["_userInitiatedPause"] = false;
 
-    const oldPause = item.pause;
+    const oldPause = item.pause.bind(item);
     item.pause = () => {
-      item['_userInitiatedPause'] = true;
+      item["_userInitiatedPause"] = true;
       oldPause();
     };
   }
@@ -205,7 +215,7 @@ export class DownloadInitiator {
     item.on("updated", this.generateItemOnUpdated());
     item.once("done", this.generateItemOnDone());
 
-    if (!item['_userInitiatedPause']) {
+    if (!item["_userInitiatedPause"]) {
       item.resume();
     }
   }

src/ElectronDownloadManager.ts

@@ -66,7 +66,7 @@ export class ElectronDownloadManager implements IElectronDownloadManager {
   resumeDownload(id: string) {
     const data = this.downloadData[id];
 
-    if (data?.item.isPaused()) {
+    if (data?.item?.isPaused()) {
       this.log(`[${id}] Resuming download`);
       data.item.resume();
     } else {
@@ -87,27 +87,31 @@ export class ElectronDownloadManager implements IElectronDownloadManager {
    *
    * Returns the id of the download.
    */
-  download(params: DownloadConfig) {
-    if (params.saveAsFilename && params.saveDialogOptions) {
-      throw new Error("You cannot define both saveAsFilename and saveDialogOptions to start a download");
-    }
-
-    const downloadInitiator = new DownloadInitiator({
-      debugLogger: this.logger,
-      onCleanup: (data) => {
-        this.cleanup(data);
-      },
+  async download(params: DownloadConfig): Promise<string> {
+    return new Promise((resolve, reject) => {
+      try {
+        if (params.saveAsFilename && params.saveDialogOptions) {
+          return reject(Error("You cannot define both saveAsFilename and saveDialogOptions to start a download"));
+        }
+
+        const downloadInitiator = new DownloadInitiator({
+          debugLogger: this.logger,
+          onCleanup: (data) => {
+            this.cleanup(data);
+          },
+          onDownloadInit: (data) => {
+            this.downloadData[data.id] = data;
+            resolve(data.id);
+          },
+        });
+
+        this.log(`[${downloadInitiator.getDownloadId()}] Registering download for url: ${truncateUrl(params.url)}`);
+        params.window.webContents.session.once("will-download", downloadInitiator.generateOnWillDownload(params));
+        params.window.webContents.downloadURL(params.url, params.downloadURLOptions);
+      } catch (e) {
+        reject(e);
+      }
     });
-
-    this.log(`[${downloadInitiator.getDownloadId()}] Registering download for url: ${truncateUrl(params.url)}`);
-    params.window.webContents.session.once("will-download", downloadInitiator.generateOnWillDownload(params));
-    params.window.webContents.downloadURL(params.url, params.downloadURLOptions);
-
-    const downloadId = downloadInitiator.getDownloadId();
-
-    this.downloadData[downloadId] = downloadInitiator.getDownloadData();
-
-    return downloadId;
   }
 
   protected cleanup(data: DownloadData) {

src/ElectronDownloadManagerMock.ts

@@ -6,7 +6,7 @@ import type { DownloadConfig, IElectronDownloadManager } from "./types";
  * that can be used for testing purposes
  */
 export class ElectronDownloadManagerMock implements IElectronDownloadManager {
-  download(_params: DownloadConfig): string {
+  async download(_params: DownloadConfig): Promise<string> {
     return "mock-download-id";
   }
 

src/__mocks__/DownloadInitiator.ts

@@ -1,24 +1,29 @@
 import { CallbackDispatcher } from "./CallbackDispatcher";
 import { DownloadData } from "./DownloadData";
 
-export const DownloadInitiator = jest.fn().mockImplementation(() => {
-  return {
+export const DownloadInitiator = jest.fn().mockImplementation((config) => {
+  const initator = {
     logger: jest.fn(),
     onItemUpdated: jest.fn(),
     onItemDone: jest.fn(),
+    onDownloadInit: jest.fn(),
     onCleanup: jest.fn(),
     callbackDispatcher: new CallbackDispatcher(),
     downloadData: new DownloadData(),
     config: { callbacks: {} },
     log: jest.fn(),
     getDownloadId: jest.fn(),
     getDownloadData: jest.fn(),
-    generateOnWillDownload: jest.fn(() => jest.fn()),
+    generateOnWillDownload: jest.fn(() => async () => {
+      config.onDownloadInit(new DownloadData());
+    }),
     initSaveAsInteractiveDownload: jest.fn(),
     initNonInteractiveDownload: jest.fn(),
     generateItemOnUpdated: jest.fn(),
     generateItemOnDone: jest.fn(),
     cleanup: jest.fn(),
     updateProgress: jest.fn(),
   };
+
+  return initator;
 });

src/types.ts

@@ -126,7 +126,7 @@ export interface IElectronDownloadManager {
    *
    * Returns the id of the download.
    */
-  download(params: DownloadConfig): string;
+  download(params: DownloadConfig): Promise<string>;
   /**
    * Cancels a download
    */

test-app/.editorconfig

@@ -0,0 +1,9 @@
+root = true
+
+[*]
+charset = utf-8
+indent_style = space
+indent_size = 2
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true

test-app/.eslintignore

@@ -0,0 +1,4 @@
+node_modules
+dist
+out
+.gitignore

test-app/.eslintrc.js

@@ -0,0 +1,7 @@
+module.exports = {
+  extends: [
+    'eslint:recommended',
+    '@electron-toolkit/eslint-config-ts/recommended',
+    '@electron-toolkit/eslint-config-prettier'
+  ]
+}

test-app/.gitignore

@@ -0,0 +1,5 @@
+node_modules
+dist
+out
+.DS_Store
+*.log*

test-app/.prettierignore

@@ -0,0 +1,6 @@
+out
+dist
+pnpm-lock.yaml
+LICENSE.md
+tsconfig.json
+tsconfig.*.json

test-app/.prettierrc.yaml

@@ -0,0 +1,4 @@
+singleQuote: true
+semi: false
+printWidth: 100
+trailingComma: none

test-app/.vscode/extensions.json

@@ -0,0 +1,3 @@
+{
+  "recommendations": ["dbaeumer.vscode-eslint"]
+}

test-app/.vscode/launch.json

@@ -0,0 +1,39 @@
+{
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "Debug Main Process",
+      "type": "node",
+      "request": "launch",
+      "cwd": "${workspaceRoot}",
+      "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/electron-vite",
+      "windows": {
+        "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/electron-vite.cmd"
+      },
+      "runtimeArgs": ["--sourcemap"],
+      "env": {
+        "REMOTE_DEBUGGING_PORT": "9222"
+      }
+    },
+    {
+      "name": "Debug Renderer Process",
+      "port": 9222,
+      "request": "attach",
+      "type": "chrome",
+      "webRoot": "${workspaceFolder}/src/renderer",
+      "timeout": 60000,
+      "presentation": {
+        "hidden": true
+      }
+    }
+  ],
+  "compounds": [
+    {
+      "name": "Debug All",
+      "configurations": ["Debug Main Process", "Debug Renderer Process"],
+      "presentation": {
+        "order": 1
+      }
+    }
+  ]
+}

test-app/.vscode/settings.json

@@ -0,0 +1,11 @@
+{
+  "[typescript]": {
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  },
+  "[javascript]": {
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  },
+  "[json]": {
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  }
+}

test-app/README.md

@@ -0,0 +1,34 @@
+# test-app
+
+A minimal Electron application with TypeScript
+
+## Recommended IDE Setup
+
+- [VSCode](https://code.visualstudio.com/) + [ESLint](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) + [Prettier](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode)
+
+## Project Setup
+
+### Install
+
+```bash
+$ yarn
+```
+
+### Development
+
+```bash
+$ yarn dev
+```
+
+### Build
+
+```bash
+# For windows
+$ yarn build:win
+
+# For macOS
+$ yarn build:mac
+
+# For Linux
+$ yarn build:linux
+```