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

Fix: proper FW revision offline error handling #16859

Merged
merged 3 commits into from
Feb 6, 2025
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
28 changes: 17 additions & 11 deletions packages/connect/src/device/checkFirmwareRevision.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@ import { FirmwareRelease, VersionArray } from '../types';
import { calculateRevisionForDevice } from './calculateRevisionForDevice';
import { FirmwareRevisionCheckError, FirmwareRevisionCheckResult } from '../types/device';

/*
* error names that signify unavailable internet connection, see https://github.com/node-fetch/node-fetch/blob/main/docs/ERROR-HANDLING.md
* Only works in Suite Desktop, where `cross-fetch` uses `node-fetch` (nodeJS environment)
* In Suite Web, the errors are unfortunately indistinguishable from other errors, because they are all lumped as CORS errors
const isNodeJSNetworkError = (e: Error) => ['FetchError', 'AbortError'].includes(e.name);
const isReactNativeNetworkError = (e: Error) =>
e.name === 'TypeError' && e.message.includes('Network request failed');

/**
* Check if an error signifies a missing fetch response (meaning network connection loss or unavailable host).
* This can only by correctly identified in nodeJS or React native runtimes (i.e. Suite Desktop main process, or Suite Lite).
* In browser runtime (Suite Web), all fetch errors are lumped together as CORS errors, therefore indistinguishable.
* (even a request that had no response is CORS error, since a non-existent response does not have CORS headers)
*/
const NODE_FETCH_OFFLINE_ERROR_NAMES = ['FetchError', 'AbortError'] as const;
const isNetworkError = (e: unknown): boolean => {
if (!(e instanceof Error)) return false;

return isNodeJSNetworkError(e) || isReactNativeNetworkError(e);
};

type GetOnlineReleaseMetadataParams = {
firmwareVersion: VersionArray;
Expand Down Expand Up @@ -94,12 +102,10 @@ export const checkFirmwareRevision = async ({
}

return { success: true };
} catch (e) {
if (NODE_FETCH_OFFLINE_ERROR_NAMES.includes(e.name)) {
return failFirmwareRevisionCheck('cannot-perform-check-offline');
}

return failFirmwareRevisionCheck('other-error');
} catch (e: unknown) {
return isNetworkError(e)
? failFirmwareRevisionCheck('cannot-perform-check-offline')
: failFirmwareRevisionCheck('other-error');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { FIRMWARE } from '@trezor/connect';
import { getFirmwareVersion } from '@trezor/device-utils';
import { isArrayMember } from '@trezor/utils';

import { hashCheckErrorScenarios } from 'src/constants/suite/firmware';
import { hashCheckErrorScenarios, revisionCheckErrorScenarios } from 'src/constants/suite/firmware';
import { useDevice, useSelector } from 'src/hooks/suite';
import { selectFirmwareRevisionCheckError } from 'src/reducers/suite/suiteReducer';
import { captureSentryMessage, withSentryScope } from 'src/utils/suite/sentry';
Expand Down Expand Up @@ -60,7 +60,7 @@ const useReportRevisionCheck = () => {
const errorType = useSelector(selectFirmwareRevisionCheckError);

useEffect(() => {
if (errorType !== null) {
if (errorType !== null && revisionCheckErrorScenarios[errorType].shouldReport) {
reportCheckFail('Firmware revision', { ...commonData, errorType });
}
}, [commonData, errorType]);
Expand Down
4 changes: 2 additions & 2 deletions packages/suite/src/constants/suite/firmware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type BehaviorBaseType = { shouldReport: boolean; debugOnly?: boolean };
// will be ignored completely
type SkippedBehavior = BehaviorBaseType & { type: 'skipped' };
// display a warning banner
type SoftWarningBehavior = BehaviorBaseType & { type: 'softWarning'; shouldReport: true };
type SoftWarningBehavior = BehaviorBaseType & { type: 'softWarning' };
// display "Device Compromised" modal, after closing it display a warning banner, block receiving address
type HardModalBehavior = BehaviorBaseType & { type: 'hardModal'; shouldReport: true };

Expand All @@ -24,7 +24,7 @@ type HashCheckErrorScenarios = Record<FirmwareHashCheckError, HashErrorBehavior>
export const revisionCheckErrorScenarios = {
'revision-mismatch': { type: 'hardModal', shouldReport: true },
'firmware-version-unknown': { type: 'hardModal', shouldReport: true },
'cannot-perform-check-offline': { type: 'softWarning', shouldReport: true },
'cannot-perform-check-offline': { type: 'softWarning', shouldReport: false },
'other-error': { type: 'softWarning', shouldReport: true },
} satisfies RevisionCheckErrorScenarios;

Expand Down
4 changes: 2 additions & 2 deletions suite-native/device/src/config/firmware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FirmwareRevisionCheckError } from '@trezor/connect';
*/

// display a warning banner
type SoftWarningBehavior = { type: 'softWarning'; shouldReport: true };
type SoftWarningBehavior = { type: 'softWarning'; shouldReport: boolean };
// display "Device Compromised" modal, after closing it dispaly a warning banner, block receiving address
type HardModalBehavior = { type: 'hardModal'; shouldReport: true };

Expand All @@ -16,6 +16,6 @@ type RevisionCheckErrorScenarios = Record<FirmwareRevisionCheckError, RevisionEr
export const revisionCheckErrorScenarios = {
'revision-mismatch': { type: 'hardModal', shouldReport: true },
'firmware-version-unknown': { type: 'hardModal', shouldReport: true },
'cannot-perform-check-offline': { type: 'softWarning', shouldReport: true },
'cannot-perform-check-offline': { type: 'softWarning', shouldReport: false },
'other-error': { type: 'softWarning', shouldReport: true },
} satisfies RevisionCheckErrorScenarios;
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as Sentry from '@sentry/react-native';
import { selectSelectedDevice } from '@suite-common/wallet-core';
import { getFirmwareVersion } from '@trezor/device-utils';

import { revisionCheckErrorScenarios } from '../config/firmware';
import { selectFirmwareRevisionCheckError } from '../selectors';

const reportCheckFail = (checkType: 'Firmware revision', contextData: any) => {
Expand All @@ -31,7 +32,10 @@ export const useReportDeviceCompromised = () => {
const revisionCheckError = useSelector(selectFirmwareRevisionCheckError);

useEffect(() => {
if (revisionCheckError !== null) {
if (
revisionCheckError !== null &&
revisionCheckErrorScenarios[revisionCheckError].shouldReport
) {
reportCheckFail('Firmware revision', { ...commonData, revisionCheckError });
}
}, [commonData, revisionCheckError]);
Expand Down
Loading