Check for invalid 'RELEASE' values in update script

[vindard]

This adds two checks for invalid RELEASE values that can potentially come from the update-status.json file:

• check that RELEASE is not an empty value. This can happen if for example the update script was run when the update-status.json file is in its default success state and the "updateTo" value is an empty string

• check that RELEASE points to a valid URL to avoid any further error from attempting to do operations on an invalid curl call

---

Error details

These changes would help the update script to not change the update-status.json to an invalid Downloading... state if Ln106 runs and the script fails, which then blocks the dashboard from loading permanently (see issue #205).

on unexpected update script run

• the update script errors as follows

  

• the update status file changes from 'success' to 'installing'

  

• the dashboard becomes inaccessible and shows this Downloading message incorrectly instead

  

[vindard]

Update

In response to this comment on the issue, I still haven't been able to replicate the in-flight update failure.

Given this though, I tried a try-catch of sorts on the entire update script with a catch script to place the update-status.json file back to a consistent state and cleanup up any dangling update dirs/files if the update script errors in any way. All changes are entirely contained in this commit and can be backed out or changed if the approach isn't ideal

---

Edit 1: On testing I found that the update-status.json did not play well with the front-end transitions if the catch is triggered

It looks like some front-end work will need to be done as well if we wanted to implement that catch. ~I've removed those commits and placed them on a separate branch for now (diff with this branch).~ Correct version of these changes were merged back into this branch.

---

Edit 2: it also looks like this try-catch should include some of the rollback plans from issue #84

---

Edit 3: I also just discovered bash's trap and I suspect that would be a cleaner way to implement this if we decide we want to go that way 🙈

[mayankchhabra]

@vindard this is awesome! It also saves our faces in case we accidentally merge an updated info.json with the new release version, but forget to tag and release the tarball on GitHub lol.

I really like your catch script. We've been thinking of implementing trap for better error handling like you pointed out, but haven't had a chance to look at it.

On testing I found that the update-status.json did not play well with the front-end transitions if the catch is triggered

If you change "state": "errored" to "state": "failed" here it would work 🎉

In my testing it worked flawlessly. The only change I had to make was to move the curl check inside the following if block:

https://github.com/getumbrel/umbrel/blob/f17c563ed948c34c98daf47a74f6b3b6eb0db4fe/scripts/update/update#L111

[vindard]

In my testing it worked flawlessly. The only change I had to make was to move the curl check inside the following if block:

Oh ya true, only want that running when update_type is "ota" :see_no_evil:

Awesome, glad those changes worked out. I went ahead and:

• made the ~errored~failed change
• pulled across the catch script back into this branch, and
• made that curl_check move change

This branch should be good to go for testing again now I think :v:

---

Also with the trap, when I checked it out some more it looked like it's used more for being more versatile with the types of errors/exits you can catch. I'm guessing the generic || version I implemented here might be ok as a generic catch for what we're trying to do with the update script?

Let me know if not though and I can check out the trap some more and implement that instead

[mayankchhabra]

Thanks a lot for working on this @vindard! As you might have noticed, quite a lot has changed recently with Umbrel 0.5 😅. Closing this PR now, and my apologies for the resulting unproductive hard work. :(

[vindard]

Thanks a lot for working on this @vindard! As you might have noticed, quite a lot has changed recently with Umbrel 0.5 😅. Closing this PR now, and my apologies for the resulting unproductive hard work. :(

Lol! Ya that would've been one hell of a merge conflict to resolve 😂