Automatic updates seem to never happen

[binki]

I have a script which I updated on 2024-04 but one of my browsers says it last updated it on 2023-05-22. Automatic updates is on and everything.

When investigating, I looked at the code in the debugger. I think I found two issues.

Lack of updateNextAt value

greasemonkey/src/bg/updater.js

Line 104 in c0b3b54

if (!nextTime || !nextUuid || v < nextTime) {

I also found that only one of my scripts had a updateNextAt. value set in chrome.storage.local.get().

It seems that v < nextTime will evaluate to false when v is undefined and nextTime is some positive number:

> undefined < 10
false

I think that the intention is that any script without a updateNextAt. value automatically be scheduled for an immediate update check. What the actual current logic appears to be is that any script which had ever successfully updated in the past will update. It seems impossible for more than one script to ever have an updateNextAt. value set for it unless if the user debugs the extension and manually adds keys. So I think that either v === undefined should be added to the list of short-circuiting conditions or v should be coalesced to 0 when its value is not present in chrome.storage.local.

To explain in more detail: it seems that on one of my systems, only one script gets updated due to this issue. On another system, only 7 of 14 or so eligible scripts gets updated. The reason that sometimes multiple scripts actually can get their updateNextAt. value set depends on the order that script UUIDs are added to the updateNextAtKeys variable. As long as a newly added script which does not have an updateNextAt. value yet is the first key in updateNextAtKeys during at least one auto-update check, then it will, as the author intended, be considered eligible for updating and join the list of scripts which are capable of auto-updating (due to successfully having its updateNextAt. value set during that update check).

Lack of Error Tolerance

At

greasemonkey/src/bg/updater.js

Line 131 in c0b3b54

await checkForUpdate(nextUuid);

I see a call to a function which can throw (i.e., awaiting a Promise which can be rejected). In the above example, when checking the values on one of my computers, the one updateNextAt. value I saw in the system was set to a value in the past. Since this is running on a browser which might be left open for weeks on a computer which does sometimes enter a low power state, I suspect, but cannot prove, that the update checks of even that one script ended up paused due to updates never being rescheduled after a download failure. I think a try{}finally{} construct is in store here. This would enable the extension to continue checking for updates even if the scheduled update occurs when the system is offline.

Please let me know if PRs would be helpful.

Thanks.

[arantius]

Your diagnosis above sounds good. PRs would be happily reviewed and probably merged. Thank you! To make that easier:

• If v being undefined is a/the problem, I'd probably prefer to change || v < nextTime to || !v || v < nextTime. With a comment about handling the case where v is undefined.
• Agreed about catching exceptions! scheduleNextScriptAutoUpdate() should always happen so finally seems called for.

[binki]

I provided the PRs.

I was wondering if the whole defaultCheckTime logic is intentional. That prevents updates from ever working if you never leave your browser open for at least 3 hours after the extension loads. All of the other update logic appears to handle short-lived browser sessions well. This is where I wonder if v could just be set to 0 when it is undefined.

Anyway, I am looking forward to your reviews!

[binki]

The more I touched the code, I start thinking about more refactoring. For example, _pickNextScriptAutoUpdate is async but still uses the Promise() constructor. And the first check for updates always fails because it happens before the script registry is loaded. Also, I expect that if the script download fails (I have a script hosted in a private repository, so updates for that will only work if I am logged into Bitbucket) the update checker will get stuck on that script and no longer check updates on other scripts (so maybe its updateNextAt. should be updated, but by a smaller amount, even after a download failure). I don’t know if I will find the time, but I might try to provide additional PRs to address those (especially the download-failure-related issue).

[arantius]

I believe #3191 and #3192 resolve this?