From 94780f1f31facf55663e81ed59d9dc9e3358acd6 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Mon, 1 Jul 2024 17:05:32 -0500 Subject: [PATCH] Fix error handling in QR code screen Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- _locales/en/messages.json | 22 ++- images/icons/v3/open/open-compact-bold.svg | 1 + images/icons/v3/refresh/refresh-bold.svg | 1 + .../InstallScreenQrCodeNotScannedStep.scss | 56 +++++++- .../InstallScreenErrorStep.stories.tsx | 4 - .../installScreen/InstallScreenErrorStep.tsx | 4 - ...tallScreenQrCodeNotScannedStep.stories.tsx | 43 +++++- .../InstallScreenQrCodeNotScannedStep.tsx | 133 ++++++++++++++---- ts/state/smart/InstallScreen.tsx | 54 +++++-- 9 files changed, 253 insertions(+), 65 deletions(-) create mode 100644 images/icons/v3/open/open-compact-bold.svg create mode 100644 images/icons/v3/refresh/refresh-bold.svg diff --git a/_locales/en/messages.json b/_locales/en/messages.json index fa65c304907..a231cac7328 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -1488,7 +1488,27 @@ }, "icu:Install__qr-failed-load": { "messageformat": "The QR code couldn't load. Check your internet and try again. Retry", - "description": "Shown on the install screen if the QR code fails to load" + "description": "(Deleted 2024/07/01) Shown on the install screen if the QR code fails to load" + }, + "icu:Install__qr-failed-load__error--timeout": { + "messageformat": "The QR code couldn't load. Check your connection and try again.", + "description": "Shown on the install screen if the QR code fails to load due to connection timeout" + }, + "icu:Install__qr-failed-load__error--unknown": { + "messageformat": "An unexpected error occurred.Please try again.", + "description": "Shown on the install screen if the QR code fails to load due to an unknown error" + }, + "icu:Install__qr-failed-load__error--network": { + "messageformat": "Signal cannot link this device using your current network.", + "description": "Shown on the install screen if the QR code fails to load due to a network error" + }, + "icu:Install__qr-failed-load__retry": { + "messageformat": "Retry", + "description": "Text of the button shown on the install screen if the QR code fails to load" + }, + "icu:Install__qr-failed-load__get-help": { + "messageformat": "Get help", + "description": "Text of the link to support page shown on the install screen if the QR code fails to load" }, "icu:Install__support-link": { "messageformat": "Need help?", diff --git a/images/icons/v3/open/open-compact-bold.svg b/images/icons/v3/open/open-compact-bold.svg new file mode 100644 index 00000000000..3c49e192fc1 --- /dev/null +++ b/images/icons/v3/open/open-compact-bold.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/images/icons/v3/refresh/refresh-bold.svg b/images/icons/v3/refresh/refresh-bold.svg new file mode 100644 index 00000000000..27a8e9577f4 --- /dev/null +++ b/images/icons/v3/refresh/refresh-bold.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/stylesheets/components/InstallScreenQrCodeNotScannedStep.scss b/stylesheets/components/InstallScreenQrCodeNotScannedStep.scss index f9536329c1c..14e84c41684 100644 --- a/stylesheets/components/InstallScreenQrCodeNotScannedStep.scss +++ b/stylesheets/components/InstallScreenQrCodeNotScannedStep.scss @@ -33,7 +33,7 @@ $size: 256px; align-items: center; - border: 2px solid transparent; + border: 1.5px solid transparent; box-sizing: content-box; display: flex; flex-direction: column; @@ -51,13 +51,31 @@ &--load-failed { @include font-subtitle; border-color: $color-gray-05; - border-radius: 4px; + border-radius: 10px; color: $color-gray-60; } &__link { @include button-reset; + @include font-body-2-bold; + + display: flex; + gap: 4px; + align-items: center; + color: $color-ultramarine; + margin-block-start: 16px; + + &::before { + @include color-svg( + '../images/icons/v3/refresh/refresh-bold.svg', + $color-ultramarine + ); + content: ''; + display: block; + height: 16px; + width: 16px; + } } &__code { @@ -69,6 +87,7 @@ &__error-message { text-align: center; + @include font-body-2; &::before { @include color-svg( @@ -78,14 +97,39 @@ content: ''; display: block; height: 22px; - margin-block: 8px 0; + margin-block: 0 8px; margin-inline: auto; width: 22px; } - a { - color: $color-ultramarine; - text-decoration: none; + margin-inline: 24px; + } + + &__error-message p { + margin-block: 0; + } + + &__get-help { + @include font-body-2-bold; + + display: flex; + gap: 4px; + align-items: center; + + margin-block-start: 16px; + + color: $color-ultramarine; + text-decoration: none; + + &::before { + @include color-svg( + '../images/icons/v3/open/open-compact-bold.svg', + $color-ultramarine + ); + content: ''; + display: block; + height: 16px; + width: 16px; } } } diff --git a/ts/components/installScreen/InstallScreenErrorStep.stories.tsx b/ts/components/installScreen/InstallScreenErrorStep.stories.tsx index 4cddc8307b9..be060db3dd6 100644 --- a/ts/components/installScreen/InstallScreenErrorStep.stories.tsx +++ b/ts/components/installScreen/InstallScreenErrorStep.stories.tsx @@ -42,7 +42,3 @@ export const _ConnectionFailed = (): JSX.Element => ( error={InstallError.ConnectionFailed} /> ); - -export const _UnknownError = (): JSX.Element => ( - -); diff --git a/ts/components/installScreen/InstallScreenErrorStep.tsx b/ts/components/installScreen/InstallScreenErrorStep.tsx index ff1c0184d4a..a2e4b9df1cf 100644 --- a/ts/components/installScreen/InstallScreenErrorStep.tsx +++ b/ts/components/installScreen/InstallScreenErrorStep.tsx @@ -15,7 +15,6 @@ export enum InstallError { TooManyDevices, TooOld, ConnectionFailed, - UnknownError, QRCodeFailed, } @@ -52,9 +51,6 @@ export function InstallScreenErrorStep({ case InstallError.ConnectionFailed: errorMessage = i18n('icu:installConnectionFailed'); break; - case InstallError.UnknownError: - errorMessage = i18n('icu:installUnknownError'); - break; case InstallError.QRCodeFailed: buttonText = i18n('icu:Install__learn-more'); errorMessage = i18n('icu:installUnknownError'); diff --git a/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.stories.tsx b/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.stories.tsx index 95edd975000..1da4400e07e 100644 --- a/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.stories.tsx +++ b/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.stories.tsx @@ -10,7 +10,10 @@ import enMessages from '../../../_locales/en/messages.json'; import type { Loadable } from '../../util/loadable'; import { LoadingState } from '../../util/loadable'; import type { PropsType } from './InstallScreenQrCodeNotScannedStep'; -import { InstallScreenQrCodeNotScannedStep } from './InstallScreenQrCodeNotScannedStep'; +import { + InstallScreenQrCodeNotScannedStep, + LoadError, +} from './InstallScreenQrCodeNotScannedStep'; const i18n = setupI18n('en', enMessages); @@ -34,8 +37,14 @@ export default { argTypes: {}, } satisfies Meta; -function Simulation({ finalResult }: { finalResult: Loadable }) { - const [provisioningUrl, setProvisioningUrl] = useState>({ +function Simulation({ + finalResult, +}: { + finalResult: Loadable; +}) { + const [provisioningUrl, setProvisioningUrl] = useState< + Loadable + >({ loadingState: LoadingState.Loading, }); @@ -83,7 +92,7 @@ export function QrCodeFailedToLoad(): JSX.Element { i18n={i18n} provisioningUrl={{ loadingState: LoadingState.LoadFailed, - error: new Error('uh oh'), + error: LoadError.Unknown, }} updates={DEFAULT_UPDATES} OS="macOS" @@ -112,12 +121,34 @@ export function SimulatedLoading(): JSX.Element { return ; } -export function SimulatedFailure(): JSX.Element { +export function SimulatedUnknownError(): JSX.Element { return ( + ); +} + +export function SimulatedNetworkIssue(): JSX.Element { + return ( + + ); +} + +export function SimulatedTimeout(): JSX.Element { + return ( + ); diff --git a/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.tsx b/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.tsx index 943e922b00e..7aeb3f5781c 100644 --- a/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.tsx +++ b/ts/components/installScreen/InstallScreenQrCodeNotScannedStep.tsx @@ -2,7 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only import type { ReactElement, ReactNode } from 'react'; -import React from 'react'; +import React, { useCallback } from 'react'; import classNames from 'classnames'; import type { LocalizerType } from '../../types/Util'; @@ -20,12 +20,18 @@ import { getClassNamesFor } from '../../util/getClassNamesFor'; import type { UpdatesStateType } from '../../state/ducks/updates'; import { Environment, getEnvironment } from '../../environment'; +export enum LoadError { + Timeout = 'Timeout', + Unknown = 'Unknown', + NetworkIssue = 'NetworkIssue', +} + // We can't always use destructuring assignment because of the complexity of this props // type. export type PropsType = Readonly<{ i18n: LocalizerType; - provisioningUrl: Loadable; + provisioningUrl: Loadable; hasExpired?: boolean; updates: UpdatesStateType; currentVersion: string; @@ -38,6 +44,9 @@ const getQrCodeClassName = getClassNamesFor( 'module-InstallScreenQrCodeNotScannedStep__qr-code' ); +const SUPPORT_PAGE = + 'https://support.signal.org/hc/articles/360007320451#desktop_multiple_device'; + export function InstallScreenQrCodeNotScannedStep({ currentVersion, hasExpired, @@ -105,7 +114,7 @@ export function InstallScreenQrCodeNotScannedStep({ {getEnvironment() !== Environment.Staging ? ( - + {i18n('icu:Install__support-link')} ) : ( @@ -118,7 +127,10 @@ export function InstallScreenQrCodeNotScannedStep({ } function InstallScreenQrCode( - props: Loadable & { i18n: LocalizerType; retryGetQrCode: () => void } + props: Loadable & { + i18n: LocalizerType; + retryGetQrCode: () => void; + } ): ReactElement { const { i18n } = props; @@ -128,33 +140,58 @@ function InstallScreenQrCode( contents = ; break; case LoadingState.LoadFailed: - contents = ( - - ) => ( - - ), - }} - /> - - ); + switch (props.error) { + case LoadError.Timeout: + contents = ( + <> + + {i18n('icu:Install__qr-failed-load__error--timeout')} + + + + ); + break; + case LoadError.Unknown: + contents = ( + <> + + + + + + ); + break; + case LoadError.NetworkIssue: + contents = ( + <> + + {i18n('icu:Install__qr-failed-load__error--network')} + + + + {i18n('icu:Install__qr-failed-load__get-help')} + + + ); + break; + default: + throw missingCaseError(props.error); + } break; case LoadingState.Loaded: contents = ( @@ -183,3 +220,37 @@ function InstallScreenQrCode( ); } + +function RetryButton({ + i18n, + onClick, +}: { + i18n: LocalizerType; + onClick: () => void; +}): JSX.Element { + const onKeyDown = useCallback( + (ev: React.KeyboardEvent) => { + if (ev.key === 'Enter') { + ev.preventDefault(); + ev.stopPropagation(); + onClick(); + } + }, + [onClick] + ); + + return ( + + ); +} + +function Paragraph(children: React.ReactNode): JSX.Element { + return

{children}

; +} diff --git a/ts/state/smart/InstallScreen.tsx b/ts/state/smart/InstallScreen.tsx index d8265851439..5e9849758a0 100644 --- a/ts/state/smart/InstallScreen.tsx +++ b/ts/state/smart/InstallScreen.tsx @@ -21,6 +21,7 @@ import { InstallScreenStep, } from '../../components/InstallScreen'; import { InstallError } from '../../components/installScreen/InstallScreenErrorStep'; +import { LoadError } from '../../components/installScreen/InstallScreenQrCodeNotScannedStep'; import { MAX_DEVICE_NAME_LENGTH } from '../../components/installScreen/InstallScreenChoosingDeviceNameStep'; import { WidthBreakpoint } from '../../components/_util'; import { HTTPError } from '../../textsecure/Errors'; @@ -44,7 +45,7 @@ type StateType = } | { step: InstallScreenStep.QrCodeNotScanned; - provisioningUrl: Loadable; + provisioningUrl: Loadable; } | { step: InstallScreenStep.ChoosingDeviceName; @@ -67,25 +68,33 @@ const qrCodeBackOff = new BackOff([ 60 * SECOND, ]); -function getInstallError(err: unknown): InstallError { +function classifyError( + err: unknown +): { installError: InstallError } | { loadError: LoadError } { if (err instanceof HTTPError) { switch (err.code) { case -1: - return InstallError.ConnectionFailed; + if ( + isRecord(err.cause) && + err.cause.code === 'SELF_SIGNED_CERT_IN_CHAIN' + ) { + return { loadError: LoadError.NetworkIssue }; + } + return { installError: InstallError.ConnectionFailed }; case 409: - return InstallError.TooOld; + return { installError: InstallError.TooOld }; case 411: - return InstallError.TooManyDevices; + return { installError: InstallError.TooManyDevices }; default: - return InstallError.UnknownError; + return { loadError: LoadError.Unknown }; } } // AccountManager.registerSecondDevice uses this specific "websocket closed" error // message. if (isRecord(err) && err.message === 'websocket closed') { - return InstallError.ConnectionFailed; + return { installError: InstallError.ConnectionFailed }; } - return InstallError.UnknownError; + return { loadError: LoadError.Unknown }; } export const SmartInstallScreen = memo(function SmartInstallScreen() { @@ -255,8 +264,13 @@ export const SmartInstallScreen = memo(function SmartInstallScreen() { ); const sleepMs = qrCodeBackOff.getAndIncrement(); log.info(`InstallScreen/getQRCode: race to ${sleepMs}ms`); - await pTimeout(qrCodeResolution.promise, sleepMs, sleepError); - await qrCodePromise; + await Promise.all([ + pTimeout(qrCodeResolution.promise, sleepMs, sleepError), + + // Note that `registerSecondDevice` resolves once the registration + // is fully complete and thus should not be subjected to a timeout. + qrCodePromise, + ]); window.IPC.removeSetupMenuItems(); } catch (error) { @@ -280,12 +294,26 @@ export const SmartInstallScreen = memo(function SmartInstallScreen() { if (error === sleepError) { setState({ step: InstallScreenStep.QrCodeNotScanned, - provisioningUrl: { loadingState: LoadingState.LoadFailed, error }, + provisioningUrl: { + loadingState: LoadingState.LoadFailed, + error: LoadError.Timeout, + }, + }); + return; + } + const classifiedError = classifyError(error); + if ('installError' in classifiedError) { + setState({ + step: InstallScreenStep.Error, + error: classifiedError.installError, }); } else { setState({ - step: InstallScreenStep.Error, - error: getInstallError(error), + step: InstallScreenStep.QrCodeNotScanned, + provisioningUrl: { + loadingState: LoadingState.LoadFailed, + error: classifiedError.loadError, + }, }); } }