Page MenuHomePhabricator

[desktop] Handle autoUpdate errors
ClosedPublic

Authored by michal on Mar 10 2023, 8:25 AM.
Tags
None
Referenced Files
F3517199: D7032.id23785.diff
Sun, Dec 22, 4:37 PM
F3516990: D7032.id23688.diff
Sun, Dec 22, 4:01 PM
Unknown Object (File)
Fri, Dec 6, 9:46 PM
Unknown Object (File)
Fri, Dec 6, 6:02 AM
Unknown Object (File)
Fri, Dec 6, 6:02 AM
Unknown Object (File)
Fri, Dec 6, 6:02 AM
Unknown Object (File)
Wed, Dec 4, 7:08 PM
Unknown Object (File)
Wed, Dec 4, 3:21 AM
Subscribers

Details

Summary

We want to handle autoUpdate errors (in particular error about no network connection). If we add a listener to an error event electron no longer displays the native alert with the error. Now we can just log it and restart the update schedule.

This replaces D6683 and D6684 as it's a much better solution.

Test Plan

Run the app, turn of internet connection. Check if the error is displayed and the app continues to check for updates.

Diff Detail

Repository
rCOMM Comm
Branch
michal/electron-fixups
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

desktop/src/auto-update.js
29

Docs says: "Emitted when there is an error while updating."

I am not sure what exactly can cause an error but are we confident that this callback will be called only after checkForUpdates() failure and we that don't need to clear timeout?

I am afraid of causing an update twice when an error was thrown from a different reason than checkForUpdates failure.

Good point, I've added a check that makes sure that the timer doesn't already exist before creating a new one

kamil added inline comments.
desktop/src/auto-update.js
18–19 ↗(On Diff #23688)

Will be safer in this order

This revision is now accepted and ready to land.Mar 13 2023, 7:23 AM