getumbrel / umbrel-middleware

RESTful Bitcoin and Lightning API for Umbrel
MIT License
21 stars 11 forks source link

Loading screen can potentially get stuck #52

Closed vindard closed 4 years ago

vindard commented 4 years ago

The problem

image

User expected behaviour:

Actual behaviour:


Troubleshooting

I found this from accidentally (I think) breaking lnd on an umbrel-dev deploy. As an un-related issue, my lnd docker image hadn't started and wasn't currently running in the environment

image

On trying to load the loading screen, I ended up stuck at 80% with the vue app polling umbrel every 2 secs and producing the following logs

image

I expect it's because the following error is being thrown in the lnd status check and I guess my questions are:

https://github.com/getumbrel/umbrel-middleware/blob/fb870f493276f27721e164f010ebf01563ca6fb1/logic/lightning.js#L770

nolim1t commented 4 years ago

This looks like an umbrel-dev issue

vindard commented 4 years ago

This looks like an umbrel-dev issue

Underlying lnd error is almost certainly an umbrel-dev issue

I like how that one failure gave a nice edge case for testing the dashboard robustness though in an unrelated way. I'd say if you guys decide this is edge enough, you can probably slap it with a low-priority label and leave it untackled for now until it either becomes a problem or stays undiscovered for long enough that it's no longer an issue :v:

mayankchhabra commented 4 years ago

Thanks for reporting this, @vindard! Great observations overall.

The dashboard calculates the loading progress by verifying if all the services are up and running (here's the logic).

Now:

for the frontend is this an issue, or are you all expecting lnd to always be setup correctly?

Yes, it's an issue, although we're also expecting lnd to be always set up correctly as both the lnd version and lnd.conf are deterministically installed. But in cases such as database corruption, or network failure, or something totally unexpected such as in your case, it's an issue because then we have no way to notify the user what went wrong.

can this likely happen if another error causes the line below to be thrown (same with other status checks)?

Yes, looks like that's where it is thrown, or else it wouldn't error out in umbrel-middleware's logs. I'm not sure if it's due to another problem though as both could be related to the same issue of lnd not starting up due to an "invalid argument".

if this can legitimately happen, what should the desired behavior be from a UX standpoint?

Good question! Since we can't login to the dashboard with any of the services not being up, a better UX would be to replace the progress bar with an error that shows what went wrong as we do not expect the users to be manually going through their logs.

Although, for the time being (beta), we can add a timeout on the loading screen that shows a tip on the likes of Hmm... it's taking unusually long to start Umbrel. You can check out the logs at http://umbrel.local/logs (where we currently tail /var/log/syslog).

vindard commented 4 years ago

@mayankchhabra glad it was helpful!

Yea that final suggestion sounds ideal for now, also maybe with some text in general on the different loading steps (once you guys aren't reticulating splines 😂 )?

I saw there was some commented code in the Vue app I think for that progress text already

mayankchhabra commented 4 years ago

Closing this now since we added the loading text :)