From 5643bbe5bcdbf7ab64d1b4d996b43f8090fbf55a Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Mon, 22 Jul 2024 08:56:32 -0400 Subject: [PATCH] feat(app): "Ignore this error" during Error Recovery (#15719) Closes EXEC-461 --- .../RecoveryOptions/FillWellAndSkip.tsx | 10 ++++------ .../RecoveryOptions/SkipStepNewTips.tsx | 10 ++++------ .../RecoveryOptions/SkipStepSameTips.tsx | 10 ++++------ .../__tests__/FillWellAndSkip.test.tsx | 9 --------- .../__tests__/SkipStepNewTips.test.tsx | 9 --------- .../__tests__/SkipStepSameTips.test.tsx | 10 +--------- .../hooks/__tests__/useRecoveryCommands.test.ts | 10 ++++------ .../hooks/useRecoveryCommands.ts | 17 ++++++----------- 8 files changed, 23 insertions(+), 62 deletions(-) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx index 5f2f8971d0a..fab5d36f8eb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/FillWellAndSkip.tsx @@ -86,7 +86,7 @@ export function SkipToNextStep( proceedToRouteAndStep, } = routeUpdateActions const { selectedRecoveryOption } = currentRecoveryOptionUtils - const { skipFailedCommand, resumeRun } = recoveryCommands + const { skipFailedCommand } = recoveryCommands const { ROBOT_SKIPPING_STEP, IGNORE_AND_SKIP } = RECOVERY_MAP const { t } = useTranslation('error_recovery') @@ -100,11 +100,9 @@ export function SkipToNextStep( } const primaryBtnOnClick = (): Promise => { - return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE) - .then(() => skipFailedCommand()) - .then(() => { - resumeRun() - }) + return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => { + skipFailedCommand() + }) } const buildBodyText = (): JSX.Element => { diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx index d8c837b33bc..33c0f199cd8 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepNewTips.tsx @@ -48,17 +48,15 @@ export function SkipStepNewTips( export function SkipStepWithNewTips(props: RecoveryContentProps): JSX.Element { const { recoveryCommands, routeUpdateActions } = props - const { skipFailedCommand, resumeRun } = recoveryCommands + const { skipFailedCommand } = recoveryCommands const { setRobotInMotion } = routeUpdateActions const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP const { t } = useTranslation('error_recovery') const primaryBtnOnClick = (): Promise => { - return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE) - .then(() => skipFailedCommand()) - .then(() => { - resumeRun() - }) + return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => { + skipFailedCommand() + }) } const buildBodyText = (): JSX.Element => { diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx index 9a679ba8d17..aed84372ccf 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/SkipStepSameTips.tsx @@ -29,17 +29,15 @@ export function SkipStepSameTips(props: RecoveryContentProps): JSX.Element { export function SkipStepSameTipsInfo(props: RecoveryContentProps): JSX.Element { const { routeUpdateActions, recoveryCommands } = props - const { skipFailedCommand, resumeRun } = recoveryCommands + const { skipFailedCommand } = recoveryCommands const { setRobotInMotion } = routeUpdateActions const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP const { t } = useTranslation('error_recovery') const primaryBtnOnClick = (): Promise => { - return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE) - .then(() => skipFailedCommand()) - .then(() => { - resumeRun() - }) + return setRobotInMotion(true, ROBOT_SKIPPING_STEP.ROUTE).then(() => { + skipFailedCommand() + }) } const buildBodyText = (): JSX.Element => { diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx index 541081f889a..123bbb33626 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/FillWellAndSkip.test.tsx @@ -141,14 +141,12 @@ describe('SkipToNextStep', () => { let mockGoBackPrevStep: Mock let mockProceedToRouteAndStep: Mock let mockSkipFailedCommand: Mock - let mockResumeRun: Mock beforeEach(() => { mockSetRobotInMotion = vi.fn(() => Promise.resolve()) mockGoBackPrevStep = vi.fn() mockProceedToRouteAndStep = vi.fn() mockSkipFailedCommand = vi.fn(() => Promise.resolve()) - mockResumeRun = vi.fn() props = { ...mockRecoveryContentProps, @@ -159,7 +157,6 @@ describe('SkipToNextStep', () => { } as any, recoveryCommands: { skipFailedCommand: mockSkipFailedCommand, - resumeRun: mockResumeRun, } as any, } }) @@ -197,15 +194,9 @@ describe('SkipToNextStep', () => { await waitFor(() => { expect(mockSkipFailedCommand).toHaveBeenCalled() }) - await waitFor(() => { - expect(mockResumeRun).toHaveBeenCalled() - }) expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan( mockSkipFailedCommand.mock.invocationCallOrder[0] ) - expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan( - mockResumeRun.mock.invocationCallOrder[0] - ) }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx index c7c9b6d27f2..afb8dfdee48 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepNewTips.test.tsx @@ -124,12 +124,10 @@ describe('SkipStepWithNewTips', () => { let props: React.ComponentProps let mockSetRobotInMotion: Mock let mockSkipFailedCommand: Mock - let mockResumeRun: Mock beforeEach(() => { mockSetRobotInMotion = vi.fn(() => Promise.resolve()) mockSkipFailedCommand = vi.fn(() => Promise.resolve()) - mockResumeRun = vi.fn() props = { ...mockRecoveryContentProps, @@ -138,7 +136,6 @@ describe('SkipStepWithNewTips', () => { } as any, recoveryCommands: { skipFailedCommand: mockSkipFailedCommand, - resumeRun: mockResumeRun, } as any, } }) @@ -169,15 +166,9 @@ describe('SkipStepWithNewTips', () => { await waitFor(() => { expect(mockSkipFailedCommand).toHaveBeenCalled() }) - await waitFor(() => { - expect(mockResumeRun).toHaveBeenCalled() - }) expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan( mockSkipFailedCommand.mock.invocationCallOrder[0] ) - expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan( - mockResumeRun.mock.invocationCallOrder[0] - ) }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx index 1b6cd3c275e..3459ce305dd 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SkipStepSameTips.test.tsx @@ -72,12 +72,10 @@ describe('SkipStepSameTipsInfo', () => { let props: React.ComponentProps let mockSetRobotInMotion: Mock let mockSkipFailedCommand: Mock - let mockResumeRun: Mock beforeEach(() => { mockSetRobotInMotion = vi.fn(() => Promise.resolve()) mockSkipFailedCommand = vi.fn(() => Promise.resolve()) - mockResumeRun = vi.fn() props = { ...mockRecoveryContentProps, @@ -86,7 +84,6 @@ describe('SkipStepSameTipsInfo', () => { } as any, recoveryCommands: { skipFailedCommand: mockSkipFailedCommand, - resumeRun: mockResumeRun, } as any, } }) @@ -114,18 +111,13 @@ describe('SkipStepSameTipsInfo', () => { RECOVERY_MAP.ROBOT_SKIPPING_STEP.ROUTE ) }) + await waitFor(() => { expect(mockSkipFailedCommand).toHaveBeenCalled() }) - await waitFor(() => { - expect(mockResumeRun).toHaveBeenCalled() - }) expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan( mockSkipFailedCommand.mock.invocationCallOrder[0] ) - expect(mockSkipFailedCommand.mock.invocationCallOrder[0]).toBeLessThan( - mockResumeRun.mock.invocationCallOrder[0] - ) }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts index 8ecb68ef459..d75387a99d4 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts @@ -221,25 +221,23 @@ describe('useRecoveryCommands', () => { ) }) - it('should call skipFailedCommand and resolve after a timeout', async () => { + it('should call skipFailedCommand and show success toast on success', async () => { const { result } = renderHook(() => useRecoveryCommands({ runId: mockRunId, failedCommand: mockFailedCommand, failedLabwareUtils: mockFailedLabwareUtils, routeUpdateActions: mockRouteUpdateActions, - recoveryToastUtils: {} as any, + recoveryToastUtils: { makeSuccessToast: mockMakeSuccessToast } as any, }) ) - const consoleSpy = vi.spyOn(console, 'log') - await act(async () => { await result.current.skipFailedCommand() }) - expect(consoleSpy).toHaveBeenCalledWith('SKIPPING TO NEXT STEP') - expect(result.current.skipFailedCommand()).resolves.toBeUndefined() + expect(mockResumeRunFromRecovery).toHaveBeenCalledWith(mockRunId) + expect(mockMakeSuccessToast).toHaveBeenCalled() }) it('should call ignoreErrorKindThisRun and resolve immediately', async () => { diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts index e7a7964e13d..3ef8f5b3809 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts @@ -29,8 +29,8 @@ export interface UseRecoveryCommandsResult { resumeRun: () => void /* A terminal recovery command that causes ER to exit as the run status becomes "stop-requested" */ cancelRun: () => void - /* A non-terminal recovery command, but should generally be chained with a resumeRun. */ - skipFailedCommand: () => Promise + /* A terminal recovery command, that causes ER to exit as the run status becomes "running" */ + skipFailedCommand: () => void /* A non-terminal recovery command. Ignore this errorKind for the rest of this run. */ ignoreErrorKindThisRun: () => Promise /* A non-terminal recovery command */ @@ -111,16 +111,11 @@ export function useRecoveryCommands({ stopRun(runId) }, [runId]) - // TODO(jh, 06-18-24): If this command is actually terminal for error recovery, remove the resumeRun currently promise - // chained where this is used. Also update docstring in iface. - const skipFailedCommand = React.useCallback((): Promise => { - console.log('SKIPPING TO NEXT STEP') - return new Promise(resolve => { - setTimeout(() => { - resolve() - }, 2000) + const skipFailedCommand = React.useCallback((): void => { + void resumeRunFromRecovery(runId).then(() => { + makeSuccessToast() }) - }, []) + }, [runId, resumeRunFromRecovery, makeSuccessToast]) const ignoreErrorKindThisRun = React.useCallback((): Promise => { console.log('IGNORING ALL ERRORS OF THIS KIND THIS RUN')