From 569a2a1409c77c1d62f6fff7f356f9617dfddf9d Mon Sep 17 00:00:00 2001 From: Alexandre Magaud Date: Wed, 8 Jun 2022 18:01:26 +0200 Subject: [PATCH] refactor(llc): useOnboardingStatePolling uses getOnboardingStatePolling --- .../hooks/useOnboardingStatePolling.test.ts | 325 +++++++++++------- .../hooks/useOnboardingStatePolling.ts | 235 ++----------- 2 files changed, 223 insertions(+), 337 deletions(-) diff --git a/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.test.ts b/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.test.ts index af269df29963..d02f7f7f2339 100644 --- a/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.test.ts +++ b/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.test.ts @@ -1,25 +1,18 @@ -import { from, TimeoutError } from "rxjs"; +import { timer, of } from "rxjs"; +import { map, delayWhen } from "rxjs/operators"; import { renderHook, act } from "@testing-library/react-hooks"; import { DeviceModelId } from "@ledgerhq/devices"; import { useOnboardingStatePolling } from "./useOnboardingStatePolling"; -import { withDevice } from "../../hw/deviceAccess"; -import getVersion from "../../hw/getVersion"; import { extractOnboardingState, OnboardingState, SeedPhraseType, OnboardingStep, } from "../../hw/extractOnboardingState"; -import Transport from "@ledgerhq/hw-transport"; -import { - DeviceOnboardingStatePollingError, - DeviceExtractOnboardingStateError, -} from "@ledgerhq/errors"; - -jest.mock("../../hw/deviceAccess"); -jest.mock("../../hw/getVersion"); -jest.mock("../../hw/extractOnboardingState"); -jest.mock("@ledgerhq/hw-transport"); +import { DisconnectedDevice } from "@ledgerhq/errors"; +import { getOnboardingStatePolling } from "../../hw/getOnboardingStatePolling"; + +jest.mock("../../hw/getOnboardingStatePolling"); jest.useFakeTimers(); const aDevice = { @@ -29,27 +22,13 @@ const aDevice = { wired: false, }; -// As extractOnboardingState is mocked, the firmwareInfo -// returned by getVersion does not matter -const aFirmwareInfo = { - isBootloader: false, - rawVersion: "", - targetId: 0, - mcuVersion: "", - flags: Buffer.from([]), -}; - const pollingPeriodMs = 1000; -const mockedGetVersion = jest.mocked(getVersion); -// const mockedWithDevice = withDevice as jest.Mock; -const mockedWithDevice = jest.mocked(withDevice); -mockedWithDevice.mockReturnValue((job) => from(job(new Transport()))); - -const mockedExtractOnboardingState = jest.mocked(extractOnboardingState); +const mockedGetOnboardingStatePolling = jest.mocked(getOnboardingStatePolling); describe("useOnboardingStatePolling", () => { let anOnboardingState: OnboardingState; + let aSecondOnboardingState: OnboardingState; beforeEach(() => { anOnboardingState = { @@ -59,184 +38,268 @@ describe("useOnboardingStatePolling", () => { currentSeedWordIndex: 0, currentOnboardingStep: OnboardingStep.NewDevice, }; + + aSecondOnboardingState = { + ...anOnboardingState, + currentOnboardingStep: OnboardingStep.NewDeviceConfirming, + }; }); afterEach(() => { - mockedGetVersion.mockClear(); - mockedExtractOnboardingState.mockClear(); + mockedGetOnboardingStatePolling.mockClear(); }); - describe("When polling returns incorrect information on the device state", () => { - it("should return a null onboarding state, continue the polling and keep track of the error", async () => { - mockedGetVersion.mockResolvedValue(aFirmwareInfo); - mockedExtractOnboardingState.mockImplementation(() => { - throw new DeviceExtractOnboardingStateError( - "Some incorrect device info" - ); - }); + describe("When polling returns a correct device state", () => { + beforeEach(() => { + mockedGetOnboardingStatePolling.mockReturnValue( + of( + { + onboardingState: { ...anOnboardingState }, + allowedError: null, + }, + { + onboardingState: { ...aSecondOnboardingState }, + allowedError: null, + } + ).pipe( + delayWhen((_, index) => { + // "delay" or "delayWhen" piped to a streaming source, for ex the "of" operator, will not block the next + // Observable to be streamed. They return an Observable that delays the emission of the source Observable, + // but do not create a delay in-between each emission. That's why the delay is increased by multiplying by "index". + // "concatMap" could have been used to wait for the previous Observable to complete, but + // the "index" arg given to "delayWhen" would always be 0 + return timer(index * pollingPeriodMs); + }) + ) + ); + }); + it("should update the onboarding state returned to the consumer", async () => { const device = aDevice; - const { result, waitForNextUpdate } = renderHook(() => + const { result } = renderHook(() => useOnboardingStatePolling({ device, pollingPeriodMs }) ); await act(async () => { jest.advanceTimersByTime(1); - // Waits for the hook update as we don't know how much time it should take - await waitForNextUpdate(); }); - expect(result.current.onboardingState).toBeNull(); - expect(result.current.allowedError).toBeInstanceOf( - DeviceExtractOnboardingStateError - ); expect(result.current.fatalError).toBeNull(); + expect(result.current.allowedError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); }); - }); - describe("When an error occurs during polling", () => { - describe("and the error is thrown before the defined timeout", () => { - it("should update the onboarding state to null and keep track of the error (here fatal error)", async () => { - mockedGetVersion.mockRejectedValue(new Error("Unknown error")); + it("should fetch again the state at a defined frequency and update (if new) the onboarding state returned to the consumer", async () => { + const device = aDevice; + + const { result } = renderHook(() => + useOnboardingStatePolling({ device, pollingPeriodMs }) + ); + + await act(async () => { + jest.advanceTimersByTime(1); + }); + + expect(result.current.fatalError).toBeNull(); + expect(result.current.allowedError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); + + // Next polling + await act(async () => { + jest.advanceTimersByTime(pollingPeriodMs); + }); + + expect(result.current.fatalError).toBeNull(); + expect(result.current.allowedError).toBeNull(); + expect(result.current.onboardingState).toEqual(aSecondOnboardingState); + }); + describe("and when the hook consumer stops the polling", () => { + it("should stop the polling and stop fetching the device onboarding state", async () => { const device = aDevice; + let stopPolling = false; - const { result, waitForNextUpdate } = renderHook(() => - useOnboardingStatePolling({ device, pollingPeriodMs }) + const { result, rerender } = renderHook(() => + useOnboardingStatePolling({ device, pollingPeriodMs, stopPolling }) ); await act(async () => { jest.advanceTimersByTime(1); - await waitForNextUpdate(); }); - expect(result.current.onboardingState).toBeNull(); + // Everything is normal on the first run + expect(mockedGetOnboardingStatePolling).toHaveBeenCalledTimes(1); + expect(result.current.fatalError).toBeNull(); expect(result.current.allowedError).toBeNull(); - expect(result.current.fatalError).toBeInstanceOf( - DeviceOnboardingStatePollingError - ); - }); - }); - - describe("and when a timeout occurred before the error (or the fetch took too long)", () => { - it("should update the allowed error value to notify the consumer", async () => { - mockedGetVersion.mockResolvedValue(aFirmwareInfo); - mockedExtractOnboardingState.mockReturnValue(anOnboardingState); - - const device = aDevice; + expect(result.current.onboardingState).toEqual(anOnboardingState); - const { result } = renderHook(() => - useOnboardingStatePolling({ device, pollingPeriodMs }) - ); + // The consumer stops the polling + stopPolling = true; + rerender({ device, pollingPeriodMs, stopPolling }); await act(async () => { - // Waits more than the timeout - jest.advanceTimersByTime(pollingPeriodMs + 1); + // Waits as long as we want + jest.advanceTimersByTime(10 * pollingPeriodMs); }); - expect(result.current.allowedError).toBeInstanceOf(TimeoutError); + // While the hook was rerendered, it did not call a new time getOnboardingStatePolling + expect(mockedGetOnboardingStatePolling).toHaveBeenCalledTimes(1); + // And the state should stay the same (and not be aSecondOnboardingState) expect(result.current.fatalError).toBeNull(); - expect(result.current.onboardingState).toBeNull(); + expect(result.current.allowedError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); }); }); }); - describe("When polling returns a correct device state", () => { - it("should return a correct onboarding state", async () => { - mockedGetVersion.mockResolvedValue(aFirmwareInfo); - mockedExtractOnboardingState.mockReturnValue(anOnboardingState); + describe("When an allowed error occurs while polling the device state", () => { + beforeEach(() => { + mockedGetOnboardingStatePolling.mockReturnValue( + of( + { + onboardingState: { ...anOnboardingState }, + allowedError: null, + }, + { + onboardingState: null, + allowedError: new DisconnectedDevice("An allowed error"), + }, + { + onboardingState: { ...aSecondOnboardingState }, + allowedError: null, + } + ).pipe( + delayWhen((_, index) => { + return timer(index * pollingPeriodMs); + }) + ) + ); + }); + it("should update the allowed error returned to the consumer, update the fatal error to null and keep the previous onboarding state", async () => { const device = aDevice; - const { result, waitForNextUpdate } = renderHook(() => + const { result } = renderHook(() => useOnboardingStatePolling({ device, pollingPeriodMs }) ); await act(async () => { jest.advanceTimersByTime(1); - await waitForNextUpdate(); }); + // Everything is ok on the first run expect(result.current.fatalError).toBeNull(); expect(result.current.allowedError).toBeNull(); expect(result.current.onboardingState).toEqual(anOnboardingState); - }); - it("should poll a new onboarding state after the defined period of time", async () => { - mockedGetVersion.mockResolvedValue(aFirmwareInfo); - mockedExtractOnboardingState - .mockReturnValueOnce(anOnboardingState) - .mockReturnValue({ - ...anOnboardingState, - currentOnboardingStep: OnboardingStep.NewDevice, - }); + await act(async () => { + jest.advanceTimersByTime(pollingPeriodMs); + }); + + expect(result.current.allowedError).toBeInstanceOf(DisconnectedDevice); + expect(result.current.fatalError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); + }); + it("should be able to recover once the allowed error is fixed and the onboarding state is updated", async () => { const device = aDevice; - const { result, waitForNextUpdate } = renderHook(() => + const { result } = renderHook(() => useOnboardingStatePolling({ device, pollingPeriodMs }) ); await act(async () => { - jest.advanceTimersByTime(1); - await waitForNextUpdate(); jest.advanceTimersByTime(pollingPeriodMs + 1); - await waitForNextUpdate(); }); - // Another run of the polling could have started between the waitForNextUpdate() - // and the advanceTimersByTime(pollingPeriodMs), so more than 2 calls could have happened - expect(mockedGetVersion.mock.calls.length).toBeGreaterThanOrEqual(2); + // Allowed error occured + expect(result.current.allowedError).toBeInstanceOf(DisconnectedDevice); + expect(result.current.fatalError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); + + await act(async () => { + jest.advanceTimersByTime(pollingPeriodMs); + }); + // Everything is ok on the next run expect(result.current.fatalError).toBeNull(); expect(result.current.allowedError).toBeNull(); - expect(result.current.onboardingState).toEqual(anOnboardingState); + expect(result.current.onboardingState).toEqual(aSecondOnboardingState); }); + }); - describe("and when the hook consumer stops the polling", () => { - it("should stop the polling and stop fetching the device onboarding state", async () => { - mockedGetVersion.mockResolvedValue(aFirmwareInfo); - mockedExtractOnboardingState.mockReturnValue(anOnboardingState); + describe("When a (fatal) error is thrown while polling the device state", () => { + const anOnboardingStateThatShouldNeverBeReached = { + ...aSecondOnboardingState, + }; - const device = aDevice; - let stopPolling = false; + beforeEach(() => { + mockedGetOnboardingStatePolling.mockReturnValue( + of( + { + onboardingState: { ...anOnboardingState }, + allowedError: null, + }, + { + onboardingState: { ...anOnboardingState }, + allowedError: null, + }, + { + // It should never be reached + onboardingState: { ...anOnboardingStateThatShouldNeverBeReached }, + allowedError: null, + } + ).pipe( + delayWhen((_, index) => { + return timer(index * pollingPeriodMs); + }), + map((value, index) => { + // Throws an error the second time + if (index === 1) { + throw new Error("An unallowed error"); + } + return value; + }) + ) + ); + }); - const { result, waitForNextUpdate, rerender } = renderHook(() => - useOnboardingStatePolling({ device, pollingPeriodMs, stopPolling }) - ); + it("should update the fatal error returned to the consumer, update the allowed error to null, keep the previous onboarding state and stop the polling", async () => { + const device = aDevice; - await act(async () => { - jest.advanceTimersByTime(1); - await waitForNextUpdate(); - }); + const { result } = renderHook(() => + useOnboardingStatePolling({ device, pollingPeriodMs }) + ); - // Everything is normal on the first run - expect(mockedGetVersion).toHaveBeenCalledTimes(1); - expect(result.current.fatalError).toBeNull(); - expect(result.current.allowedError).toBeNull(); - expect(result.current.onboardingState).toEqual(anOnboardingState); + await act(async () => { + jest.advanceTimersByTime(1); + }); - // The consumer stops the polling - stopPolling = true; - rerender({ device, pollingPeriodMs, stopPolling }); - // If another run of the polling started, it will max terminated in pollingPeriodMs + // Everything is ok on the first run + expect(result.current.fatalError).toBeNull(); + expect(result.current.allowedError).toBeNull(); + expect(result.current.onboardingState).toEqual(anOnboardingState); + + await act(async () => { jest.advanceTimersByTime(pollingPeriodMs); - mockedGetVersion.mockClear(); - mockedGetVersion.mockResolvedValue(aFirmwareInfo); + }); - await act(async () => { - // Waits as long as we want - jest.advanceTimersByTime(10 * pollingPeriodMs); - }); + // Fatal error on the second run + expect(result.current.allowedError).toBeNull(); + expect(result.current.fatalError).toBeInstanceOf(Error); + expect(result.current.onboardingState).toEqual(anOnboardingState); - // No polling should occur - expect(mockedGetVersion).toHaveBeenCalledTimes(0); - // And the state should stay the same - expect(result.current.fatalError).toBeNull(); - expect(result.current.allowedError).toBeNull(); - expect(result.current.onboardingState).toEqual(anOnboardingState); + await act(async () => { + jest.advanceTimersByTime(pollingPeriodMs); }); + + // The polling should have been stopped, and we never update the onboardingState + expect(result.current.allowedError).toBeNull(); + expect(result.current.fatalError).toBeInstanceOf(Error); + expect(result.current.onboardingState).not.toEqual( + anOnboardingStateThatShouldNeverBeReached + ); }); }); }); diff --git a/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.ts b/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.ts index 753f0f7166a5..ab6b8ea3b917 100644 --- a/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.ts +++ b/libs/ledger-live-common/src/onboarding/hooks/useOnboardingStatePolling.ts @@ -1,43 +1,18 @@ import { useState, useEffect } from "react"; -import { - from, - merge, - partition, - of, - throwError, - Observable, - Subscription, - TimeoutError, -} from "rxjs"; -import { map, catchError, repeat, first, timeout } from "rxjs/operators"; -import getVersion from "../../hw/getVersion"; -import { withDevice } from "../../hw/deviceAccess"; +import { Subscription } from "rxjs"; import type { Device } from "../../hw/actions/types"; -import { - TransportStatusError, - DeviceOnboardingStatePollingError, - DeviceExtractOnboardingStateError, - DisconnectedDevice, - CantOpenDevice, -} from "@ledgerhq/errors"; -import { FirmwareInfo } from "../../types/manager"; -import { - extractOnboardingState, - OnboardingState, -} from "../../hw/extractOnboardingState"; +import { DeviceOnboardingStatePollingError } from "@ledgerhq/errors"; -export type OnboardingStatePollingResult = { - onboardingState: OnboardingState | null; - allowedError: Error | null; -}; +import type { OnboardingStatePollingResult } from "../../hw/getOnboardingStatePolling"; +import { getOnboardingStatePolling } from "../../hw/getOnboardingStatePolling"; +import { OnboardingState } from "../../hw/extractOnboardingState"; export type UseOnboardingStatePollingResult = OnboardingStatePollingResult & { fatalError: Error | null; }; -// Polls the current device onboarding state -// TODO dependency injection withDevice (and getVersion ?) to easily test ? -// or dependency injection in onboardingStatePolling ? +// Polls the current device onboarding state, and notify the hook consumer of +// any allowed errors and fatal errors export const useOnboardingStatePolling = ({ device, pollingPeriodMs, @@ -47,12 +22,9 @@ export const useOnboardingStatePolling = ({ pollingPeriodMs: number; stopPolling?: boolean; }): UseOnboardingStatePollingResult => { - const [onboardingStatePollingResult, setOnboardingStatePollingResult] = - useState({ - onboardingState: null, - allowedError: null, - }); - + const [onboardingState, setOnboardingState] = + useState(null); + const [allowedError, setAllowedError] = useState(null); const [fatalError, setFatalError] = useState(null); useEffect(() => { @@ -65,7 +37,7 @@ export const useOnboardingStatePolling = ({ `SyncOnboarding: ๐Ÿง‘โ€๐Ÿ’ป new device: ${JSON.stringify(device)}` ); - onboardingStatePollingSubscription = onboardingStatePolling({ + onboardingStatePollingSubscription = getOnboardingStatePolling({ deviceId: device.deviceId, pollingPeriodMs, }).subscribe({ @@ -76,12 +48,14 @@ export const useOnboardingStatePolling = ({ )}` ); - // Does not update the state if it could not be extracted from the flags if (onboardingStatePollingResult) { - console.log( - "SyncOnboarding: onboarding state from polling is not null" - ); - setOnboardingStatePollingResult(onboardingStatePollingResult); + setFatalError(null); + setAllowedError(onboardingStatePollingResult.allowedError); + + // Does not update the onboarding state if an allowed error occurred + if (!onboardingStatePollingResult.allowedError) { + setOnboardingState(onboardingStatePollingResult.onboardingState); + } } }, error: (error) => { @@ -90,6 +64,7 @@ export const useOnboardingStatePolling = ({ error, })}` ); + setAllowedError(null); setFatalError( new DeviceOnboardingStatePollingError( `Error from: ${error?.name ?? error} ${error?.message}` @@ -103,166 +78,14 @@ export const useOnboardingStatePolling = ({ console.log("SyncOnboarding: cleaning up polling ๐Ÿงน"); onboardingStatePollingSubscription?.unsubscribe(); }; - }, [device, pollingPeriodMs, setOnboardingStatePollingResult, stopPolling]); - - return { ...onboardingStatePollingResult, fatalError }; -}; - -// TODO: Put in live-common/src/onboarding/onboardingStatePolling ? -export const onboardingStatePolling = ({ - deviceId, - pollingPeriodMs, -}: { - deviceId: string; - pollingPeriodMs: number; -}): Observable => { - console.log("๐ŸŽ GOING TO START"); - - // Could just be a boolean: firstRun ? - let i = 0; - - const delayedOnboardingStateOnce$: Observable = - new Observable((subscriber) => { - console.log(`SyncOnboarding: โ–ถ๏ธ Polling from Observable ${i}`); - const delayMs = i > 0 ? pollingPeriodMs : 0; - console.log(`SyncOnboarding: polling delayed by ${delayMs} ms`); - i++; - - const getOnboardingStateOnce = () => { - const firmwareInfoOrAllowedError$ = withDevice(deviceId)((t) => - from(getVersion(t)) - ).pipe( - // TODO: choose timeout ms value. For now = polling period - timeout(pollingPeriodMs), // Throws a TimeoutError - first(), - catchError((error: any) => { - if (isAllowedOnboardingStatePollingError(error)) { - // Pushes the error to the next step to be processed (no retry from the beginning) - return of(error); - } - - console.log( - `SyncOnboarding: ๐Ÿ’ฅ Fatal Error ${error} -> ${JSON.stringify( - error - )}` - ); - return throwError(error); - }) - ); - - // If an error is catched previously, and this error is "allowed", - // the value from the observable is not a FirmwareInfo but an Error - const [firmwareInfo$, allowedError$] = partition( - firmwareInfoOrAllowedError$, - (value) => !!value?.flags - ); - - // Handles the case of a FirmwareInfo value - const onboardingStateFromFirmwareInfo$ = firmwareInfo$.pipe( - map((firmwareInfo: FirmwareInfo) => { - console.log( - `SyncOnboarding: โ™ง MAP got firmwareInfo: ${JSON.stringify( - firmwareInfo - )}` - ); - - let onboardingState: OnboardingState | null = null; - - try { - onboardingState = extractOnboardingState(firmwareInfo.flags); - } catch (error) { - console.log( - `SyncOnboarding: extract onboarding error ${JSON.stringify( - error - )}` - ); - if (error instanceof DeviceExtractOnboardingStateError) { - return { - onboardingState: null, - allowedError: - error as typeof DeviceExtractOnboardingStateError, - }; - } else { - return { - onboardingState: null, - allowedError: new DeviceOnboardingStatePollingError( - "SyncOnboarding: Unknown error while extracting the onboarding state" - ), - }; - } - } - return { onboardingState, allowedError: null }; - }) - ); - - // Handles the case of an (allowed) Error value - const onboardingStateFromAllowedError$ = allowedError$.pipe( - map((allowedError: Error) => { - console.log( - `SyncOnboarding: โ™ง MAP got accepted error: ${JSON.stringify( - allowedError - )}` - ); - return { - onboardingState: null, - allowedError: allowedError, - }; - }) - ); - - return merge( - onboardingStateFromFirmwareInfo$, - onboardingStateFromAllowedError$ - ); - }; - - // Delays the fetch of the onboarding state - setTimeout(() => { - getOnboardingStateOnce().subscribe({ - next: (value: OnboardingStatePollingResult) => { - subscriber.next(value); - }, - error: (error: any) => { - subscriber.error(error); - }, - complete: () => subscriber.complete(), - }); - }, delayMs); - }); - - return delayedOnboardingStateOnce$.pipe(repeat()); -}; - -// TODO: decide which errors are allowed -export const isAllowedOnboardingStatePollingError = (error: Error): boolean => { - // Timeout error thrown by rxjs's timeout - if (error && error instanceof TimeoutError) { - console.log(`SyncOnboarding: timeout error โŒ›๏ธ ${JSON.stringify(error)}`); - return true; - } - - if (error && error instanceof DisconnectedDevice) { - console.log( - `SyncOnboarding: disconnection error ๐Ÿ”Œ ${JSON.stringify(error)}` - ); - return true; - } - - if (error && error instanceof CantOpenDevice) { - console.log( - `SyncOnboarding: cannot open device error ๐Ÿ”Œ ${JSON.stringify(error)}` - ); - return true; - } - - if ( - error && - error instanceof TransportStatusError - // error.statusCode === 0x6d06 - ) { - console.log(`SyncOnboarding: 0x6d06 error ๐Ÿ”จ ${JSON.stringify(error)}`); - return true; - } - - return false; + }, [ + device, + pollingPeriodMs, + setOnboardingState, + setAllowedError, + setFatalError, + stopPolling, + ]); + + return { onboardingState, allowedError, fatalError }; };