fix(connect): FW revision check offline handling #15664
Conversation
| @@ -87,7 +95,7 @@ export const checkFirmwareRevision = async ({ | |||
|
|
|||
| return { success: true }; | |||
| } catch (e) { | |||
| if (e.name === 'FetchError' && e.code === 'ENOTFOUND') { | |||
There was a problem hiding this comment.
This catches only when network adapter is on, but there is no response or 4xx or 5xx response from server.
This did not catch error when network adapter is off. And that's is not an edge case - laptop/mobile users can routinely toggle Wi-Fi to save battery power...
Handling it in getOnlineReleaseMetadata makes sure we catch any fetch error, while still distinguishing from other-error in main function body.
Lemonexe
requested review from
mroz22,
szymonlesisz,
martykan and
marekrjpolak
as code owners
| isEqual(onlineRelease.version, firmwareVersion), | ||
| ); | ||
| } catch { | ||
| throw OFFLINE_ERROR; |
There was a problem hiding this comment.
This will hide original error.
There was a problem hiding this comment.
Also I do not really like throwing something what is not the exception (in JS the Error)
There was a problem hiding this comment.
The original implementation did not use the error either 🤷
It only reported failFirmwareRevisionCheck('other-error') or failFirmwareRevisionCheck('cannot-perform-check-offline')
There was a problem hiding this comment.
Great innitiative, but unfortunately, it reintroduces the bug I attempted to fix in #14626.
I reproduced it by using a T3B1 device, removing 2.8.3 from releases.json, and changing the URL to https://raw.githubusercontent.com/trezor/data/d704edf1dfff1fe97226af706ac8eff6291c6bb8/firmware/t3b1/releases.json` - this leads to a JSON which cannot be parsed.
As a result, I see this banner despite being online:
Right. Meanwhile I also noticed when looking into the failing unit test, that the current behavior was done intentionally. |
Description
Fix handling of fetch errors when FW revision check encounters version not known locally.
You can test locally with user env
1-main | 2-mainand turning off Wi-Fi.Or you can start a released emulator and delete the corresponding entry from
releases.json, and try it with Wi-Fi on/off.Screenshots:
Before:
After:
