-
-
Notifications
You must be signed in to change notification settings - Fork 455
Safer Updater #1762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safer Updater #1762
Conversation
…o avoid error to crash the main thread
This comment has been minimized.
This comment has been minimized.
|
Done an update test to ensure working as expected? |
This comment has been minimized.
This comment has been minimized.
|
Can you please fix spelling errors 'Uodate', 'settigns'. |
98d855c to
dfe2c01
Compare
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view or the 📜action log for details. Unrecognized words (4)autoupdater To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands... in a clone of the [email protected]:Flow-Launcher/Flow.Launcher.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/3862695136/attempts/1'To have the bot do this for you, reply quoting the following line: If the flagged items are 🤯 false positivesIf items relate to a ...
|
| { | ||
| Log.Exception($"|Updater.UpdateApp|Check your connection and proxy settings to github-cloud.s3.amazonaws.com.", e); | ||
| if ((e is HttpRequestException or WebException or SocketException || e.InnerException is TimeoutException)) | ||
| Log.Exception($"|Updater.UpdateApp|Check your connection and proxy settings to github-cloud.s3.amazonaws.com.", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this, I wonder if we need to notify users when these type of errors occur?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we did? In the later part when silentupdate is false
| // check update every 5 hours | ||
| var timer = new PeriodicTimer(TimeSpan.FromHours(5)); | ||
| await _updater.UpdateAppAsync(API); | ||
|
|
||
| while (await timer.WaitForNextTickAsync()) | ||
| // check updates on startup | ||
| await _updater.UpdateAppAsync(API); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a double up of await _updater.UpdateAppAsync(API);? Doesnt it run once timer is instantiated?
|
@taooceros have you tested updating with this change? |
I can't reproduce the original error,but since I only add a try catch case it should be safe. |
might close #1504
Note:
I am not so sure why the autoupdater may crash flow itself...I guess it is because the Elapsed callback is triggered from the main thread, but I cannot find evidence to support it, because based on document, this happens only when SynchronizingObject is set which is not we have done.
Though, I have suppressed the error of general exception from updater, so now at least it will be safer to use the autoupdater.