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

feat(app, react-api-client): add "retry failed command" during Error Recovery #15170

Merged
merged 7 commits into from
May 21, 2024
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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff this against the old index.tsx to see changes.

The differences are the new route for the "retrying command" view, the useInitialPipetteHome, and the new name, ErrorRecoveryWizard.

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))
}, [])
}
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
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
Loading