tiny-pilot / tinypilot

Use your Raspberry Pi as a browser-based KVM.
https://tinypilotkvm.com
MIT License
2.99k stars 250 forks source link

Improve error messaging when updates fail due to an incorrectly set clock #1653

Closed cghague closed 10 months ago

cghague commented 10 months ago

Resolves #1503.

This change adds code to intercept CERTIFICATE_VERIFY_FAILED errors that have a reason of "certificate is not yet valid" when checking for updates so the app can show a more actionable error message. Any other type of certificate or URL error is unaffected.

You can test the change by turning off the network time service and then setting the date back by a few years:

sudo systemctl stop systemd-timesyncd
sudo date --set='-2 years'

Review on CodeApprove

mtlynch commented 10 months ago

Cool, good progress.

I tested it out, and here's what I'm seeing:

image

I think this is more informative, but I don't think it's quite actionable.

We're telling the user to check the logs, but that in itself doesn't solve the problem. Even if they see that the time is incorrect, they likely won't know what to do to fix the issue.

What I'd like the user to see is something more like this:

Unable to Connect to TinyPilot Update Server

Your TinyPilot device's date differs signifcantly from the date on the TinyPilot update server. This can happen if your TinyPilot device does not have Internet access or your network blocks access to time servers (NTP servers).

Wait for up to five minutes to give your TinyPilot device time to synchronize its clock, and then try updating again.

Details:

Error: Server's certificate start date is ahead of TinyPilot's system date of 2023-10-11. Check the system date to ensure that it is accurate. <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate is not yet valid (_ssl.c:1123)>

I'm avoiding discussing implementation details to give you a chance to stretch your skills here, but if more guidance will be helpful, @jotaen4tinypilot can be a helpful resource here.

mtlynch commented 10 months ago
Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request

cghague commented 10 months ago

Thanks for the feedback, @mtlynch!

I considered implementing a more detailed error screen but incorrectly assumed the desired outcome was limited to making the existing error text more relevant. In hindsight, the mention of JavaScript in the issue comment should have been a clue!

I've looked at how this is handled elsewhere in the app, and I think I can see how to do it, but I'll be sure to reach out to @jotaen4tinypilot if I have any questions.

cghague commented 10 months ago

@jotaen4tinypilot - I've updated this change to display the error message described by @mtlynch in his last comment. I'm optimistic that I've taken the right approach, but I'm still new to our conventions and Python, so any feedback will be appreciated!