fix: change abort to aborted event

[beyondkmp]

There are many SimpleURLLoaderWrapper server error in sentrty, I think they might be caused by this event.

Here's a simple example:

// test.js
'use strict';

// The same issue occurs with `http`, too.
const https = require('https');

const req = https.request('https://nodejs.org/dist/v13.7.0/node-v13.7.0.tar.gz');
req.on('response', (res) => {
  res
    .on('end', () => console.log('end'))
    .on('close', () => console.log('close'))
    .on('aborted', () => console.log('aborted'))
    .on('error', (err) => console.log(err));

  setTimeout(() => {
    console.log('start');
    req.abort();
  }, 500);
});
req.end();

[changeset-bot]

🦋 Changeset detected

Latest commit: 3cb8446

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
electron-updater	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for car-park-attendant-cleat-11576 ready!

Name	Link
🔨 Latest commit	3cb8446
🔍 Latest deploy log	https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/66968621d2a89000080b74e9
😎 Deploy Preview	https://deploy-preview-8282--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[beyondkmp]

Here's a minimal example, with an 'aborted' event. However, I'm currently not sure why this event isn't documented.

// test.js
'use strict';

// The same issue occurs with `http`, too.
const https = require('https');

const req = https.request('https://nodejs.org/dist/v13.7.0/node-v13.7.0.tar.gz');
req.on('response', (res) => {
  res.on('aborted', () => console.log('aborted'))
  res.on('abort', () => console.log('abort'))

  setTimeout(() => {
    console.log('start');
    req.abort();
  }, 500);
});
req.end();

[mmaietta]

Okay, that's super weird indeed. Thanks for the test script! Maybe we should have both listeners (just in case for backward compatibility)?

Any chance you'd be willing to post a screenshot of what error you're seeing in Sentry?

[beyondkmp]

@mmaietta Here's the detailed sentry error. Because we recently enabled differential udpate, such errors have increased, so I suspect there is a connection here.

[mmaietta]

Any chance you'd be willing to test this on your app locally using patch-package first? I don't immediately see any harm/side-effects in the changes in this PR, but I don't have a way to verify locally via unit tests AFAICT.

[beyondkmp]

Unfortunately, I cannot reproduce this issue locally at the moment. It only seems to occur frequently in production when there is a large user base. I'll have to wait for production to test it out.

[mmaietta]

@beyondkmp any luck with your patched production build rollout? Looking to determine next steps with this PR 🙂

[beyondkmp]

It hasn't been released yet. Once it is released and we have results, I will provide immediate feedback.

[beyondkmp]

@mmaietta Based on the release results, network errors have not increased; instead, they have decreased, although they have not been completely eliminated.

From the results, my suggestion is that this can be merged.