Skip to content

Commit

Permalink
feat(app, react-api-client): add "retry failed command" during Error …
Browse files Browse the repository at this point in the history
…Recovery (#15170)

Closes EXEC-432

Adds the ability to retry the command that caused the run to enter Error Recovery, resuming the run after the issued command completes. Implementing the retry logic requires laying out (hopefully) most of the remaining app-side foundational work, therefore that is included in this commit as well.
  • Loading branch information
mjhuff authored May 21, 2024
1 parent ae51aa5 commit dac2589
Show file tree
Hide file tree
Showing 30 changed files with 1,161 additions and 282 deletions.
1 change: 1 addition & 0 deletions app/src/assets/localization/en/error_recovery.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@
"run_will_resume": "The run will resume from the point at which the error occurred. Take any necessary actions to correct the problem first. If the step is completed successfully, the protocol continues.",
"stand_back": "Stand back, robot is in motion",
"stand_back_resuming": "Stand back, resuming current step",
"stand_back_retrying": "Stand back, retrying current command",
"view_recovery_options": "View recovery options"
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,12 @@ describe('ProtocolRunHeader', () => {
pause: () => {},
stop: () => {},
reset: () => {},
resumeFromRecovery: () => {},
isPlayRunActionLoading: false,
isPauseRunActionLoading: false,
isStopRunActionLoading: false,
isResetRunLoading: false,
isResumeRunFromRecoveryActionLoading: false,
})
when(vi.mocked(useRunStatus)).calledWith(RUN_ID).thenReturn(RUN_STATUS_IDLE)
when(vi.mocked(useRunTimestamps)).calledWith(RUN_ID).thenReturn({
Expand Down Expand Up @@ -777,10 +779,12 @@ describe('ProtocolRunHeader', () => {
pause: () => {},
stop: () => {},
reset: () => {},
resumeFromRecovery: () => {},
isPlayRunActionLoading: false,
isPauseRunActionLoading: false,
isStopRunActionLoading: false,
isResetRunLoading: true,
isResumeRunFromRecoveryActionLoading: false,
})
render()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ describe('HistoricalProtocolRunOverflowMenu', () => {
pause: () => {},
stop: () => {},
reset: () => {},
resumeFromRecovery: () => {},
isPlayRunActionLoading: false,
isPauseRunActionLoading: false,
isStopRunActionLoading: false,
isResetRunLoading: false,
isResumeRunFromRecoveryActionLoading: false,
})
when(useNotifyAllCommandsQuery)
.calledWith(
Expand Down
134 changes: 134 additions & 0 deletions app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import * as React from 'react'
import { createPortal } from 'react-dom'
import { useSelector } from 'react-redux'

import {
BORDERS,
COLORS,
DIRECTION_COLUMN,
Flex,
POSITION_ABSOLUTE,
} from '@opentrons/components'

import { getIsOnDevice } from '../../redux/config'
import { getModalPortalEl } from '../../App/portal'
import { BeforeBeginning } from './BeforeBeginning'
import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions'
import { ErrorRecoveryHeader } from './ErrorRecoveryHeader'
import { RecoveryInProgress } from './RecoveryInProgress'
import { getErrorKind, useRouteUpdateActions } from './utils'
import { useRecoveryCommands } from './useRecoveryCommands'
import { RECOVERY_MAP } from './constants'

import type { FailedCommand, IRecoveryMap, RecoveryContentProps } from './types'

export interface ErrorRecoveryFlowsProps {
runId: string
failedCommand: FailedCommand | null
}

export function ErrorRecoveryWizard({
runId,
failedCommand,
}: ErrorRecoveryFlowsProps): JSX.Element {
/**
* Recovery Route: A logically-related collection of recovery steps or a single step if unrelated to any existing recovery route.
* Recovery Step: Analogous to a "step" in other wizard flows.
*/
const [recoveryMap, setRecoveryMap] = React.useState<IRecoveryMap>({
route: RECOVERY_MAP.OPTION_SELECTION.ROUTE,
step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT,
})

const errorKind = getErrorKind(failedCommand?.error?.errorType)
const isOnDevice = useSelector(getIsOnDevice)
const routeUpdateActions = useRouteUpdateActions({
recoveryMap,
setRecoveryMap,
})
const recoveryCommands = useRecoveryCommands({
runId,
failedCommand,
})

useInitialPipetteHome(recoveryCommands, routeUpdateActions)

return (
<ErrorRecoveryComponent
failedCommand={failedCommand}
errorKind={errorKind}
isOnDevice={isOnDevice}
recoveryMap={recoveryMap}
routeUpdateActions={routeUpdateActions}
recoveryCommands={recoveryCommands}
/>
)
}

function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element {
return createPortal(
<Flex
flexDirection={DIRECTION_COLUMN}
width="992px"
height="568px"
left="14.5px"
top="16px"
borderRadius={BORDERS.borderRadius12}
position={POSITION_ABSOLUTE}
backgroundColor={COLORS.white}
>
<ErrorRecoveryHeader errorKind={props.errorKind} />
<ErrorRecoveryContent {...props} />
</Flex>,
getModalPortalEl()
)
}

export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element {
const buildBeforeBeginning = (): JSX.Element => {
return <BeforeBeginning {...props} />
}

const buildSelectRecoveryOption = (): JSX.Element => {
return <SelectRecoveryOption {...props} />
}

const buildRecoveryInProgress = (): JSX.Element => {
return <RecoveryInProgress {...props} />
}

const buildResumeRun = (): JSX.Element => {
return <ResumeRun {...props} />
}

switch (props.recoveryMap.route) {
case RECOVERY_MAP.BEFORE_BEGINNING.ROUTE:
return buildBeforeBeginning()
case RECOVERY_MAP.OPTION_SELECTION.ROUTE:
return buildSelectRecoveryOption()
case RECOVERY_MAP.RESUME.ROUTE:
return buildResumeRun()
case RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE:
case RECOVERY_MAP.ROBOT_RESUMING.ROUTE:
case RECOVERY_MAP.ROBOT_RETRYING_COMMAND.ROUTE:
return buildRecoveryInProgress()
default:
return buildSelectRecoveryOption()
}
}

// Home the Z-axis of all attached pipettes on Error Recovery launch.
export function useInitialPipetteHome(
recoveryCommands: ReturnType<typeof useRecoveryCommands>,
routeUpdateActions: ReturnType<typeof useRouteUpdateActions>
): void {
const { homePipetteZAxes } = recoveryCommands
const { setRobotInMotion } = routeUpdateActions

// Synchronously set the recovery route to "robot in motion" before initial render to prevent screen flicker on ER launch.
React.useLayoutEffect(() => {
void setRobotInMotion(true)
.then(() => homePipetteZAxes())
.finally(() => setRobotInMotion(false))
}, [])
}
8 changes: 7 additions & 1 deletion app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import type { RobotMovingRoute, RecoveryContentProps } from './types'
export function RecoveryInProgress({
recoveryMap,
}: RecoveryContentProps): JSX.Element {
const { ROBOT_IN_MOTION, ROBOT_RESUMING } = RECOVERY_MAP
const {
ROBOT_IN_MOTION,
ROBOT_RESUMING,
ROBOT_RETRYING_COMMAND,
} = RECOVERY_MAP
const { t } = useTranslation('error_recovery')
const { route } = recoveryMap

Expand All @@ -19,6 +23,8 @@ export function RecoveryInProgress({
return t('stand_back')
case ROBOT_RESUMING.ROUTE:
return t('stand_back_resuming')
case ROBOT_RETRYING_COMMAND.ROUTE:
return t('stand_back_retrying')
default:
return t('stand_back')
}
Expand Down
16 changes: 13 additions & 3 deletions app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,27 @@ import {
StyledText,
} from '@opentrons/components'

import { RECOVERY_MAP } from '../constants'
import { RecoveryFooterButtons } from './shared'

import type { RecoveryContentProps } from '../types'

export function ResumeRun({
isOnDevice,
onComplete,
routeUpdateActions,
recoveryCommands,
}: RecoveryContentProps): JSX.Element | null {
const { ROBOT_RETRYING_COMMAND } = RECOVERY_MAP
const { t } = useTranslation('error_recovery')
const { goBackPrevStep } = routeUpdateActions

const { retryFailedCommand, resumeRun } = recoveryCommands
const { goBackPrevStep, setRobotInMotion } = routeUpdateActions

const primaryBtnOnClick = (): Promise<void> => {
return setRobotInMotion(true, ROBOT_RETRYING_COMMAND.ROUTE) // Show the "retrying" motion screen while exiting ER.
.then(() => retryFailedCommand())
.then(() => resumeRun())
}

if (isOnDevice) {
return (
Expand Down Expand Up @@ -50,7 +60,7 @@ export function ResumeRun({
</Flex>
<RecoveryFooterButtons
isOnDevice={isOnDevice}
primaryBtnOnClick={onComplete}
primaryBtnOnClick={primaryBtnOnClick}
secondaryBtnOnClick={goBackPrevStep}
primaryBtnTextOverride={t('confirm')}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react'
import { vi, describe, it, expect, beforeEach } from 'vitest'
import { screen, fireEvent } from '@testing-library/react'
import { screen, fireEvent, waitFor } from '@testing-library/react'

import { renderWithProviders } from '../../../../__testing-utils__'
import { i18n } from '../../../../i18n'
Expand All @@ -16,20 +16,19 @@ const render = (props: React.ComponentProps<typeof ResumeRun>) => {
}

describe('RecoveryFooterButtons', () => {
const { RESUME } = RECOVERY_MAP
const { RESUME, ROBOT_RETRYING_COMMAND } = RECOVERY_MAP
let props: React.ComponentProps<typeof ResumeRun>
let mockOnComplete: Mock
let mockGoBackPrevStep: Mock

beforeEach(() => {
mockOnComplete = vi.fn()
mockGoBackPrevStep = vi.fn()
const mockRouteUpdateActions = { goBackPrevStep: mockGoBackPrevStep } as any

props = {
isOnDevice: true,
recoveryCommands: {} as any,
failedCommand: {} as any,
errorKind: ERROR_KINDS.GENERAL_ERROR,
onComplete: mockOnComplete,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: {
route: RESUME.ROUTE,
Expand All @@ -38,21 +37,68 @@ describe('RecoveryFooterButtons', () => {
}
})

it('renders appropriate copy and click behavior', () => {
it('renders appropriate copy and click behavior', async () => {
render(props)

screen.getByText('Are you sure you want to resume?')
screen.queryByText(
'The run will resume from the point at which the error occurred.'
)

const primaryBtn = screen.getByRole('button', { name: 'Confirm' })
const secondaryBtn = screen.getByRole('button', { name: 'Go back' })

fireEvent.click(primaryBtn)
fireEvent.click(secondaryBtn)

expect(mockOnComplete).toHaveBeenCalled()
expect(mockGoBackPrevStep).toHaveBeenCalled()
})

it('should call commands in the correct order for the primaryOnClick callback', async () => {
const setRobotInMotionMock = vi.fn(() => Promise.resolve())
const retryFailedCommandMock = vi.fn(() => Promise.resolve())
const resumeRunMock = vi.fn()

const mockRecoveryCommands = {
retryFailedCommand: retryFailedCommandMock,
resumeRun: resumeRunMock,
} as any

const mockRouteUpdateActions = {
setRobotInMotion: setRobotInMotionMock,
} as any

render({
...props,
recoveryCommands: mockRecoveryCommands,
routeUpdateActions: mockRouteUpdateActions,
})

const primaryBtn = screen.getByRole('button', { name: 'Confirm' })
fireEvent.click(primaryBtn)

await waitFor(() => {
expect(setRobotInMotionMock).toHaveBeenCalledTimes(1)
})
await waitFor(() => {
expect(setRobotInMotionMock).toHaveBeenCalledWith(
true,
ROBOT_RETRYING_COMMAND.ROUTE
)
})
await waitFor(() => {
expect(retryFailedCommandMock).toHaveBeenCalledTimes(1)
})
await waitFor(() => {
expect(resumeRunMock).toHaveBeenCalledTimes(1)
})

expect(setRobotInMotionMock.mock.invocationCallOrder[0]).toBeLessThan(
retryFailedCommandMock.mock.invocationCallOrder[0]
)

await waitFor(() => {
expect(retryFailedCommandMock.mock.invocationCallOrder[0]).toBeLessThan(
resumeRunMock.mock.invocationCallOrder[0]
)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ describe('SelectRecoveryOption', () => {
props = {
isOnDevice: true,
errorKind: ERROR_KINDS.GENERAL_ERROR,
onComplete: vi.fn(),
failedCommand: {} as any,
recoveryCommands: {} as any,
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: {
route: RESUME.ROUTE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ describe('BeforeBeginning', () => {

props = {
isOnDevice: true,
recoveryCommands: {} as any,
failedCommand: {} as any,
errorKind: ERROR_KINDS.GENERAL_ERROR,
onComplete: vi.fn(),
routeUpdateActions: mockRouteUpdateActions,
recoveryMap: {
route: BEFORE_BEGINNING.ROUTE,
Expand Down
Loading

0 comments on commit dac2589

Please sign in to comment.