electron-builder
electron-builder copied to clipboard
fix: change abort to aborted event
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 detected
Latest commit: 3cb844685e10563a07a64f77b343bcc4d94d1bc5
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
Deploy Preview for car-park-attendant-cleat-11576 ready!
| Name | Link |
|---|---|
| Latest commit | 3cb844685e10563a07a64f77b343bcc4d94d1bc5 |
| 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.
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();
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?
@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.
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.
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.
@beyondkmp any luck with your patched production build rollout? Looking to determine next steps with this PR 🙂
It hasn't been released yet. Once it is released and we have results, I will provide immediate feedback.
@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.