electron: SimpleURLLoaderWrapper broken with redirects in > 7.1.2

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

  • Electron Version: v7.1.4
  • Operating System: Ubuntu + MacOS
  • Last Known Working Electron version: 7.1.2

Expected Behavior

electron and electron-updater should follow redirect URLs

Actual Behavior

electron-updater / electron-builder breaks with Electron > 7.1.2

Redirect was cancelled at SimpleURLLoaderWrapper. (electron/js2c/browser_init.js:2561:23) at SimpleURLLoaderWrapper.emit (events.js:203:13)

This NUKED my app update system and I think it’s a really critical bug that needs to be fixed.

To Reproduce

I don’t have an easy way to reproduce.

You could TRY to fetch this URL:

https://github.com/burtonator/polar-bookshelf/releases/download/v1.80.3/Polar-Bookshelf-1.80.3-mac.zip

which is what the auto-updater is fetching and then breaks on.

PS … Electron rocks! Except for this issue of course.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 7
  • Comments: 18 (7 by maintainers)

Commits related to this issue

Most upvoted comments

@nornagon It doesn’t look like anyone is actively working on electron-updater. Lots of apps are using it and 0476eb67 broke update for those apps. This should probably get some priority since most app authors likely won’t notice this until thousands of users are stranded with apps that no longer update. Would be great if you can help investigate and revert your change until this bug is understood and fixed?

  1. doDownload in httpExecutor.
  2. createRequest in electronHttpExecutor with redirect: "manual".
  3. addRedirectHandlers adds on("redirect") but request.followRedirect() is never called. Instead request.abort() is called starting another doDownload with the new location.
  4. The second doDownload gets a 200 response and the file is downloaded in the right location but Error: Redirect was cancelled is raised causing the whole update to fail.
  addRedirectHandlers(request, options, reject, redirectCount, handler) {
    request.on("redirect", (statusCode, method, redirectUrl) => {

      // no way to modify request options, abort old and make a new one
      // https://github.com/electron/electron/issues/11505
      request.abort();

      if (redirectCount > this.maxRedirects) {
        reject(this.createMaxRedirectError());
      } else {
        handler(_builderUtilRuntime().HttpExecutor.prepareRedirectUrlOptions(redirectUrl, options));
      }
    });
  }
Error: Error: Redirect was cancelled
    at SimpleURLLoaderWrapper.<anonymous> (electron/js2c/browser_init.js:2561:23)
    at SimpleURLLoaderWrapper.emit (events.js:203:13)
(node:59127) UnhandledPromiseRejectionWarning: Error: Redirect was cancelled
    at SimpleURLLoaderWrapper.<anonymous> (electron/js2c/browser_init.js:2561:23)
    at SimpleURLLoaderWrapper.emit (events.js:203:13)
(node:59127) UnhandledPromiseRejectionWarning: Error: Redirect was cancelled
    at SimpleURLLoaderWrapper.<anonymous> (electron/js2c/browser_init.js:2561:23)
    at SimpleURLLoaderWrapper.emit (events.js:203:13)
(node:59127) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:59127) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:59127) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:59127) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

SimpleURLLoaderWrapper should probably not die with an error if request.abort() was called by the handler:

0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 390)     this._urlLoader.on('redirect', (event, redirectInfo, headers) => {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 391)       const { statusCode, newMethod, newUrl } = redirectInfo
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 392)       if (this._redirectPolicy === 'error') {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 393)         this._die(new Error(`Attempted to redirect, but redirect policy was 'error'`))
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 394)       } else if (this._redirectPolicy === 'manual') {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 395)         let _followRedirect = false
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 396)         this._followRedirectCb = () => { _followRedirect = true }
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 397)         try {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 398)           this.emit('redirect', statusCode, newMethod, newUrl, headers)
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 399)         } finally {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 400)           this._followRedirectCb = null
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 401)           if (!_followRedirect) {
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 402)             this._die(new Error('Redirect was cancelled'))
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 403)           }
0476eb67ab (Jeremy Apthorp 2019-12-02 12:26:47 -0800 404)         }

Electron v7.1.8 should have the fix to this issue.

@CatalanCabbage I haven’t tried it. It’s not clear to me how that solution solves that problem and I’m concerned about it creating new problems.

@lukehaas, yes! Take a look at this issue, it has a commit reference too. 😃
https://github.com/electron-userland/electron-builder/issues/4987#issuecomment-647039914

But apparently some people have had issues after that too - you can go through the mentions on that thread to check those issues too.

@nornagon thanks for pinging, but I don’t work on electron-updater and don’t use it anymore.