sindresorhus / electron-dl

Simplified file downloads for your Electron app
MIT License
1.16k stars 137 forks source link

Will two download() calls resolve the same downloadItem in some rare conditions? #44

Open reallyimeric opened 6 years ago

reallyimeric commented 6 years ago

Would two listener got the same downloadItem (one who arrived first) if they are attached to the session at the same time? Or, can we ensure the resolved downloadItem is the one of currently download call?

reallyimeric commented 6 years ago

It seems two calls got the same downloadItem.

Sample code:

win.webContents.on('did-finish-load', () => {
    const p1 = download(win, target);
    const p2 = download(win, target2);
    Promise.all([p1, p2]).then((arr) => {
        console.log(arr[0] === arr[1]);
    });
});

And more, the two listeners has been called for 4 times in total, which I think is not expected.

Maybe we should avoid calling like this?

motin commented 6 years ago

Confirmed, and it is not intermittent in my case but 100% reproducible. I attempted to set up bulk download via the code below and noticed that the downloads got tangled.

const downloadByUrl = async (url, options) => {
  // https://electronjs.org/docs/api/download-item
  const dl /* :DownloadItem */ = await download(mainWindow, url, options);
  const downloadItemInfo = {
    savePath: dl.getSavePath(),
    url: dl.getURL(),
    mimeType: dl.getMimeType(),
  };
  return downloadItemInfo;
};

interface DownloadConfig {
  url: any;
  options: any;
}

const bulkDownload = async (downloadConfigs: Array<DownloadConfig>) => {
  const promises = [];
  downloadConfigs.forEach(downloadConfig => {
    promises.push(downloadByUrl(downloadConfig.url, downloadConfig.options));
  });
  const downloadItemInfos = await Promise.all(promises);
  log.debug("downloadItemInfos", downloadItemInfos);
  return downloadItemInfos;
};

Proof of tangled downloads when trying to download another-small-text-file.txt and small-text-file.txt below.

downloadConfigs sent to bulkDownload():

[
  {
    options: {
      directory: '/var/folders/_1/fnvjsx8j2r97wknt0vvfvhmh0000gn/T/7ReCyHxuMUbBv2hIuvmn',
      filename: 'another-small-text-file.txt'
    },
    url: 'http://localhost:7777/another-small-text-file.txt'
  },
  {
    options: {
      directory: '/var/folders/_1/fnvjsx8j2r97wknt0vvfvhmh0000gn/T/7ReCyHxuMUbBv2hIuvmn',
      filename: 'small-text-file.txt'
    },
    url: 'http://localhost:7777/small-text-file.txt'
  }
]

Resulting downloadItemInfos:

[
  {
    savePath: '/var/folders/_1/fnvjsx8j2r97wknt0vvfvhmh0000gn/T/7ReCyHxuMUbBv2hIuvmn/small-text-file.txt',
    url: 'http://localhost:7777/another-small-text-file.txt',
    mimeType: 'text/plain'
  },
  {
    savePath: '/var/folders/_1/fnvjsx8j2r97wknt0vvfvhmh0000gn/T/7ReCyHxuMUbBv2hIuvmn/small-text-file.txt',
    url: 'http://localhost:7777/another-small-text-file.txt',
    mimeType: 'text/plain'
  }
]
reallyimeric commented 6 years ago

I have forgotten about this, but seems that the download call will resolve the downloadItem from will-download event listener, which would get the same downloadItem if the latter listener registered before the former will-download event dispatched.

IMO if electron-dl would like to handle this correctly, it should implement a queue mechanism to prevent/delay the latter download call/listener registration until got a will-download event, for currently electron has not provided any api to get the specified downloadItem.

ribsies commented 6 years ago

After looking at the source code, it looks like it isn't setup to easily handle multiple downloads at one time.

By calling the 'download' function multiple times you are calling the 'registerListeners' multiple times which will reset some variables that are meant to be used for multiple file downloads. It appears that function is only meant to be called once.

Hard to say exactly what the author was thinking. He needs some good examples.

In the mean time, its very easy to implement your own setup for this.

theogravity commented 7 months ago

My library handles multiple downloads.

https://github.com/theogravity/electron-dl-manager

hellodigua commented 4 months ago

six years later, it is still happen : (

xianyunleo commented 3 weeks ago

My library support Multiple downloads are handled properly and report individual progress. You are also able to cancel / pause / resume as well. Easy to use

https://github.com/xianyunleo/electron-dl-downloader