Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling in firmware revision check #14626

Merged
merged 2 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/connect/src/device/Device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,10 +570,12 @@ export class Device extends TypedEmitter<DeviceEvents> {
if (
// The check was not yet performed
this.authenticityChecks.firmwareRevision === null ||
// The check was performed, but outcome cannot be surely determined (Suite is offline)
// The check was performed, but outcome cannot be surely determined (Suite is offline or there was an unexpected error)
// -> recheck on every getFeatures() until the result is known
(this.authenticityChecks.firmwareRevision.success === false &&
this.authenticityChecks.firmwareRevision.error === 'cannot-perform-check-offline')
['cannot-perform-check-offline', 'other-error'].includes(
this.authenticityChecks.firmwareRevision.error,
))
) {
await this.checkFirmwareRevision();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { FetchError } from 'node-fetch';

import { DeviceModelInternal } from '@trezor/protobuf';
import { checkFirmwareRevision, CheckFirmwareRevisionParams } from '../checkFirmwareRevision';
import { FirmwareRelease, FirmwareRevisionCheckResult } from '../../exports';
Expand Down Expand Up @@ -85,17 +87,29 @@ describe.each(DeviceNames)(`${checkFirmwareRevision.name} for device %s`, intern
expected: { success: false, error: 'firmware-version-unknown' },
},
{
it:
'returns null (NOT SUCCESS / NOT ERROR) when firmware version is not found locally, and the user is offline' +
'In this case. User SHALL BE warned to go ONLINE and UPGRADE before doing anything!',
it: 'fails with a specific error message when the check cannot be performed because the revision is not found locally and the user is offline',
httpRequestMock: () => {
throw new Error('You are offline!');
throw new FetchError('You are offline!', 'network', {
code: 'ENOTFOUND',
name: 'FetchError',
message: 'You are offline!',
});
},
params: createDeviceParams({
expectedRevision: undefined, // firmware not known by local releases.json file
}),
expected: { success: false, error: 'cannot-perform-check-offline' },
},
{
it: 'fails with a generic error message when there is an error when reading the online version of releases.json',
httpRequestMock: () => {
throw new Error('There is an unexpected error!');
},
params: createDeviceParams({
expectedRevision: undefined, // firmware not known by local releases.json file
}),
expected: { success: false, error: 'other-error' },
},
])(`$it`, async ({ params, expected, httpRequestMock }) => {
if (httpRequestMock !== undefined) {
jest.spyOn(utilsAssets, 'httpRequest').mockImplementation(httpRequestMock);
Expand Down
15 changes: 8 additions & 7 deletions packages/connect/src/device/checkFirmwareRevision.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isEqual } from '@trezor/utils/src/versionUtils';
import { PROTO } from '../constants';
import { downloadReleasesMetadata } from '../data/downloadReleasesMetadata';
import { FirmwareRelease, VersionArray } from '../types';
import { FirmwareRevisionCheckResult } from '../types/device';
import { FirmwareRevisionCheckError, FirmwareRevisionCheckResult } from '../types/device';
import { calculateRevisionForDevice } from './calculateRevisionForDevice';

type GetOnlineReleaseMetadataParams = {
Expand All @@ -20,8 +20,8 @@ const getOnlineReleaseMetadata = async ({
};

const failFirmwareRevisionCheck = (
error: 'revision-mismatch' | 'firmware-version-unknown' | 'firmware-version-unknown',
): FirmwareRevisionCheckResult => ({ success: false, error });
error: FirmwareRevisionCheckError,
): Extract<FirmwareRevisionCheckResult, { success: false }> => ({ success: false, error });

export type CheckFirmwareRevisionParams = {
firmwareVersion: VersionArray;
Expand Down Expand Up @@ -86,10 +86,11 @@ export const checkFirmwareRevision = async ({

return { success: true };
} catch (e) {
return {
success: false,
error: 'cannot-perform-check-offline',
};
if (e.name === 'FetchError' && e.code === 'ENOTFOUND') {
return failFirmwareRevisionCheck('cannot-perform-check-offline');
}

return failFirmwareRevisionCheck('other-error');
}
}

Expand Down
12 changes: 7 additions & 5 deletions packages/connect/src/types/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ export type DeviceState = {
// key = coin shortcut lowercase (ex: btc, eth, xrp) OR field declared in coins.json "supportedFirmware.capability"
export type UnavailableCapabilities = { [key: string]: UnavailableCapability };

export type FirmwareRevisionCheckError =
| 'revision-mismatch'
| 'firmware-version-unknown'
| 'cannot-perform-check-offline' // suite offline & release version not found locally => we cannot check with `data.trezor.io`
| 'other-error'; // incorrect URL, cannot parse JSON, etc.

export type FirmwareRevisionCheckResult =
| { success: true }
| {
success: false;
error:
| 'revision-mismatch'
| 'firmware-version-unknown'
| 'firmware-version-unknown'
| 'cannot-perform-check-offline'; // suite offline & release version not found locally => we cannot check with `data.trezor.io`
error: FirmwareRevisionCheckError;
};

export type KnownDevice = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,32 @@
import { TranslationKey } from '@suite-common/intl-types';
import { isDeviceAcquired } from '@suite-common/suite-utils';
import { selectDevice } from '@suite-common/wallet-core';
import { Banner } from '@trezor/components';
import { FirmwareRevisionCheckError } from '@trezor/connect';
import { HELP_CENTER_FIRMWARE_REVISION_CHECK } from '@trezor/urls';

import { Translation, TrezorLink } from 'src/components/suite';
import { useSelector } from 'src/hooks/suite';

const messages: Record<FirmwareRevisionCheckError, TranslationKey> = {
'cannot-perform-check-offline': 'TR_DEVICE_FIRMWARE_REVISION_CHECK_UNABLE_TO_PERFORM',
'other-error': 'TR_FIRMWARE_REVISION_CHECK_OTHER_ERROR',
'revision-mismatch': 'TR_FIRMWARE_REVISION_CHECK_FAILED',
'firmware-version-unknown': 'TR_FIRMWARE_REVISION_CHECK_FAILED',
};

export const FirmwareRevisionCheckBanner = () => {
const device = useSelector(selectDevice);

if (
!isDeviceAcquired(device) ||
device.authenticityChecks?.firmwareRevision?.success !== false
) {
return null;
}

const wasOffline =
device?.features &&
device.authenticityChecks?.firmwareRevision?.success === false &&
device.authenticityChecks.firmwareRevision.error === 'cannot-perform-check-offline';
const message = wasOffline
? 'TR_DEVICE_FIRMWARE_REVISION_CHECK_UNABLE_TO_PERFORM'
: 'TR_FIRMWARE_REVISION_CHECK_FAILED';

return (
<Banner
Expand All @@ -30,7 +42,7 @@ export const FirmwareRevisionCheckBanner = () => {
)
}
>
<Translation id={message} />
<Translation id={messages[device.authenticityChecks.firmwareRevision.error]} />
</Banner>
);
};
6 changes: 4 additions & 2 deletions packages/suite/src/reducers/suite/suiteReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,10 @@ export const selectIsFirmwareRevisionCheckEnabledAndFailed = (
isDeviceAcquired(device) &&
// If `check` is null, it means that it was not performed yet.
device.authenticityChecks?.firmwareRevision?.success === false &&
// If Suite is offline and we cannot perform check, the error banner shows to urge user to go online.
device.authenticityChecks.firmwareRevision.error !== 'cannot-perform-check-offline'
// If Suite is offline and cannot perform check or there is some unexpected error, an error banner is shown but Suite is otherwise unaffected.
!['cannot-perform-check-offline', 'other-error'].includes(
device.authenticityChecks.firmwareRevision.error,
)
);
};

Expand Down
4 changes: 4 additions & 0 deletions packages/suite/src/support/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7112,6 +7112,10 @@ export default defineMessages({
defaultMessage:
"Firmware revision check couldn't be performed. Go online to verify your firmware version.",
},
TR_FIRMWARE_REVISION_CHECK_OTHER_ERROR: {
id: 'TR_FIRMWARE_REVISION_CHECK_OTHER_ERROR',
defaultMessage: 'Firmware revision check could not be performed.',
},
TR_ONBOARDING_COINS_STEP: {
id: 'TR_ONBOARDING_COINS_STEP',
defaultMessage: 'Activate coins',
Expand Down
Loading