SimpleURLLoaderWrapper broken with redirects in > 7.1.2

[burtonator]

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.

[sentialx]

Yeah, I probably lost most of my app users, because I didn't notice this bug before...

[nornagon]

Yikes, that seems bad! It looks like that error is related to the redirect mode "manual". Do you have any snippets of code that set the redirect mode to manual and/or register 'redirect' handlers? That might helpful in tracking down what's going wrong here.

[nornagon]

Looks like this is where electron-updater sets redirect to "manual": https://github.com/electron-userland/electron-builder/blob/master/packages/electron-updater/src/electronHttpExecutor.ts#L27

[nornagon]

cc @develar @vladimiry who work on electron-updater

[vladimiry]

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

[burtonator]

Looks like this is where electron-updater sets redirect to "manual": https://github.com/electron-userland/electron-builder/blob/master/packages/electron-updater/src/electronHttpExecutor.ts#L27

ouch.. sure enough. What's frustrating is that with Github all the URLs are manual so I don't even of if there's a workaround for this.

[nornagon]

Yeah... this seems like a bug in electron-updater if it doesn't always call request.followRedirect() when the redirect mode is set to "manual" and it wants to follow a redirect. However, looking at the code for the previous version of the network service pre-7.1.3, it seems like the behavior would have been the same, so maybe something else is going on?

Unfortunately it's hard to say without more info from the electron-updater side of things :/

[lyswhut]

Get this error when electron> 7.1.2
electron-update stopped working: electron-userland/electron-builder#4505

[burtonator]

@lyswhut I'm narrowing it down to it being a bug / conflict with > 7.1.2 too. I'm reverting to 7.1.2 now to hopefully fix this.

[lutzroeder]

@nornagon It doesn't look like anyone is actively working on electron-updater. Lots of apps are using it and 0476eb6 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)         }

[nornagon]

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

[agata]

I've checked it on macOS, Windows and Linux. It works fine.

• Electron v7.1.8
• electron-builder v22.2.0
• electron-updator v4.2.0

[CatalanCabbage]

I'm getting a similar error after I check for updates with electron-updater and there's an update; I'm on Electron-updater 4.3.1 and Electron 7.1.11. ☹️
This is the project if it helps: https://gitlab.com/cataxcab/the-wall
Any ideas?

[lukehaas]

@CatalanCabbage did you find a solution for this?

[paulojrab321]

Vou olhar! Em seg, 2 de ago de 2021 06:33, Luke Haas ***@***.***> escreveu:

…

@CatalanCabbage <https://github.com/CatalanCabbage> did you find a solution for this? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#21566 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVBZNK4JB2VEOXJLKMFBBALT2ZQ4XANCNFSM4J4NSDLA> .

[CatalanCabbage]

@lukehaas, yes!
Take a look at this issue, it has a commit reference too. :)
electron-userland/electron-builder#4987 (comment)

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

[CatalanCabbage]

@lukehaas did it work?

[lukehaas]

@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.