Skip to content

Commit

Permalink
fix(api-client, react-api-client, app, robot-server): support multipl…
Browse files Browse the repository at this point in the history
…e recovery policies during a run (#16950)

Closes RQA-3670

We need to support the client ignoring several different classes of errors in a single run, which means the app needs to know the current recovery policy and be able to modify. This PR adds the necessary infrastructure to support that.

Co-authored-by: Max Marrone <[email protected]>
  • Loading branch information
mjhuff and SyntaxColoring authored Nov 22, 2024
1 parent 64aaeb9 commit 7e6c8ee
Show file tree
Hide file tree
Showing 18 changed files with 396 additions and 127 deletions.
17 changes: 17 additions & 0 deletions api-client/src/runs/getErrorRecoveryPolicy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { GET, request } from '../request'

import type { HostConfig } from '../types'
import type { ResponsePromise } from '../request'
import type { ErrorRecoveryPolicyResponse } from './types'

export function getErrorRecoveryPolicy(
config: HostConfig,
runId: string
): ResponsePromise<ErrorRecoveryPolicyResponse> {
return request<ErrorRecoveryPolicyResponse>(
GET,
`/runs/${runId}/errorRecoveryPolicy`,
null,
config
)
}
1 change: 1 addition & 0 deletions api-client/src/runs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export * from './createLabwareOffset'
export * from './createLabwareDefinition'
export * from './constants'
export * from './updateErrorRecoveryPolicy'
export * from './getErrorRecoveryPolicy'

export * from './types'
export type { CreateRunData } from './createRun'
1 change: 1 addition & 0 deletions api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export interface UpdateErrorRecoveryPolicyRequest {
}

export type UpdateErrorRecoveryPolicyResponse = Record<string, never>
export type ErrorRecoveryPolicyResponse = UpdateErrorRecoveryPolicyRequest

/**
* Current Run State Data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import { renderHook, act } from '@testing-library/react'
import {
useResumeRunFromRecoveryMutation,
useStopRunMutation,
useUpdateErrorRecoveryPolicy,
useResumeRunFromRecoveryAssumingFalsePositiveMutation,
} from '@opentrons/react-api-client'

import { useChainRunCommands } from '/app/resources/runs'
import {
useChainRunCommands,
useUpdateRecoveryPolicyWithStrategy,
} from '/app/resources/runs'
import {
useRecoveryCommands,
HOME_PIPETTE_Z_AXES,
Expand Down Expand Up @@ -80,9 +82,9 @@ describe('useRecoveryCommands', () => {
vi.mocked(useChainRunCommands).mockReturnValue({
chainRunCommands: mockChainRunCommands,
} as any)
vi.mocked(useUpdateErrorRecoveryPolicy).mockReturnValue({
mutateAsync: mockUpdateErrorRecoveryPolicy,
} as any)
vi.mocked(useUpdateRecoveryPolicyWithStrategy).mockReturnValue(
mockUpdateErrorRecoveryPolicy as any
)
vi.mocked(
useResumeRunFromRecoveryAssumingFalsePositiveMutation
).mockReturnValue({
Expand Down Expand Up @@ -361,7 +363,8 @@ describe('useRecoveryCommands', () => {
)

expect(mockUpdateErrorRecoveryPolicy).toHaveBeenCalledWith(
expectedPolicyRules
expectedPolicyRules,
'append'
)
})

Expand Down
40 changes: 17 additions & 23 deletions app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import head from 'lodash/head'
import {
useResumeRunFromRecoveryMutation,
useStopRunMutation,
useUpdateErrorRecoveryPolicy,
useResumeRunFromRecoveryAssumingFalsePositiveMutation,
} from '@opentrons/react-api-client'

import { useChainRunCommands } from '/app/resources/runs'
import {
useChainRunCommands,
useUpdateRecoveryPolicyWithStrategy,
} from '/app/resources/runs'
import { DEFINED_ERROR_TYPES, ERROR_KINDS, RECOVERY_MAP } from '../constants'
import { getErrorKind } from '/app/organisms/ErrorRecoveryFlows/utils'

Expand All @@ -23,12 +25,7 @@ import type {
PrepareToAspirateRunTimeCommand,
MoveLabwareParams,
} from '@opentrons/shared-data'
import type {
CommandData,
IfMatchType,
RecoveryPolicyRulesParams,
RunAction,
} from '@opentrons/api-client'
import type { CommandData, IfMatchType, RunAction } from '@opentrons/api-client'
import type { WellGroup } from '@opentrons/components'
import type { FailedCommand, RecoveryRoute, RouteStep } from '../types'
import type { UseFailedLabwareUtilsResult } from './useFailedLabwareUtils'
Expand All @@ -38,6 +35,7 @@ import type { UseRecoveryAnalyticsResult } from '/app/redux-resources/analytics'
import type { CurrentRecoveryOptionUtils } from './useRecoveryRouting'
import type { ErrorRecoveryFlowsProps } from '..'
import type { FailedCommandBySource } from './useRetainedFailedCommandBySource'
import type { UpdateErrorRecoveryPolicyWithStrategy } from '/app/resources/runs'

interface UseRecoveryCommandsParams {
runId: string
Expand Down Expand Up @@ -100,9 +98,7 @@ export function useRecoveryCommands({
mutateAsync: resumeRunFromRecoveryAssumingFalsePositive,
} = useResumeRunFromRecoveryAssumingFalsePositiveMutation()
const { stopRun } = useStopRunMutation()
const {
mutateAsync: updateErrorRecoveryPolicy,
} = useUpdateErrorRecoveryPolicy(runId)
const updateErrorRecoveryPolicy = useUpdateRecoveryPolicyWithStrategy(runId)
const { makeSuccessToast } = recoveryToastUtils

// TODO(jh, 11-21-24): Some commands return a 200 with an error body. We should catch these and propagate the error.
Expand Down Expand Up @@ -231,10 +227,12 @@ export function useRecoveryCommands({
ifMatch
)

return updateErrorRecoveryPolicy(ignorePolicyRules)
return updateErrorRecoveryPolicy(ignorePolicyRules, 'append')
.then(() => Promise.resolve())
.catch(() =>
Promise.reject(new Error('Failed to update recovery policy.'))
.catch((e: Error) =>
Promise.reject(
new Error(`Failed to update recovery policy: ${e.message}`)
)
)
} else {
void proceedToRouteAndStep(RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE)
Expand Down Expand Up @@ -421,12 +419,8 @@ export const buildIgnorePolicyRules = (
commandType: FailedCommand['commandType'],
errorType: string,
ifMatch: IfMatchType
): RecoveryPolicyRulesParams => {
return [
{
commandType,
errorType,
ifMatch,
},
]
}
): UpdateErrorRecoveryPolicyWithStrategy['newPolicy'] => ({
commandType,
errorType,
ifMatch,
})
1 change: 1 addition & 0 deletions app/src/resources/runs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ export * from './useModuleCalibrationStatus'
export * from './useProtocolAnalysisErrors'
export * from './useLastRunCommand'
export * from './useRunStatuses'
export * from './useUpdateRecoveryPolicyWithStrategy'
60 changes: 60 additions & 0 deletions app/src/resources/runs/useUpdateRecoveryPolicyWithStrategy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import {
useHost,
useUpdateErrorRecoveryPolicy,
} from '@opentrons/react-api-client'
import { getErrorRecoveryPolicy } from '@opentrons/api-client'

import type {
HostConfig,
RecoveryPolicyRulesParams,
UpdateErrorRecoveryPolicyResponse,
} from '@opentrons/api-client'

/**
* append - Add a new policy rule to the end of the existing recovery policy.
*/
export type UpdatePolicyStrategy = 'append'

export interface UpdateErrorRecoveryPolicyWithStrategy {
runId: string
newPolicy: RecoveryPolicyRulesParams[number]
strategy: UpdatePolicyStrategy
}

export function useUpdateRecoveryPolicyWithStrategy(
runId: string
): (
newPolicy: UpdateErrorRecoveryPolicyWithStrategy['newPolicy'],
strategy: UpdateErrorRecoveryPolicyWithStrategy['strategy']
) => Promise<UpdateErrorRecoveryPolicyResponse> {
const host = useHost()

const {
mutateAsync: updateErrorRecoveryPolicy,
} = useUpdateErrorRecoveryPolicy(runId)

return (
newPolicy: UpdateErrorRecoveryPolicyWithStrategy['newPolicy'],
strategy: UpdateErrorRecoveryPolicyWithStrategy['strategy']
) =>
getErrorRecoveryPolicy(host as HostConfig, runId).then(res => {
const existingPolicyRules = res.data.data.policyRules.map(rule => ({
commandType: rule.matchCriteria.command.commandType,
errorType: rule.matchCriteria.command.error.errorType,
ifMatch: rule.ifMatch,
}))

const buildUpdatedPolicy = (): RecoveryPolicyRulesParams => {
switch (strategy) {
case 'append':
return [...existingPolicyRules, newPolicy]
default: {
console.error('Unhandled policy strategy, defaulting to append.')
return [...existingPolicyRules, newPolicy]
}
}
}

return updateErrorRecoveryPolicy(buildUpdatedPolicy())
})
}
1 change: 1 addition & 0 deletions react-api-client/src/runs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export { useRunCommandErrors } from './useRunCommandErrors'
export * from './useCreateLabwareOffsetMutation'
export * from './useCreateLabwareDefinitionMutation'
export * from './useUpdateErrorRecoveryPolicy'
export * from './useErrorRecoveryPolicy'

export type { UsePlayRunMutationResult } from './usePlayRunMutation'
export type { UsePauseRunMutationResult } from './usePauseRunMutation'
Expand Down
31 changes: 31 additions & 0 deletions react-api-client/src/runs/useErrorRecoveryPolicy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { useQuery } from 'react-query'

import { getErrorRecoveryPolicy } from '@opentrons/api-client'

import { useHost } from '../api'

import type { UseQueryOptions, UseQueryResult } from 'react-query'
import type {
ErrorRecoveryPolicyResponse,
HostConfig,
} from '@opentrons/api-client'

export function useErrorRecoveryPolicy(
runId: string,
options: UseQueryOptions<ErrorRecoveryPolicyResponse, Error> = {}
): UseQueryResult<ErrorRecoveryPolicyResponse, Error> {
const host = useHost()

const query = useQuery<ErrorRecoveryPolicyResponse, Error>(
[host, 'runs', runId, 'errorRecoveryPolicy'],
() =>
getErrorRecoveryPolicy(host as HostConfig, runId)
.then(response => response.data)
.catch(e => {
throw e
}),
options
)

return query
}
4 changes: 2 additions & 2 deletions react-api-client/src/runs/useUpdateErrorRecoveryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
HostConfig,
} from '@opentrons/api-client'

export type UseUpdateErrorRecoveryPolicyResponse = UseMutationResult<
export type UseErrorRecoveryPolicyResponse = UseMutationResult<
UpdateErrorRecoveryPolicyResponse,
AxiosError,
RecoveryPolicyRulesParams
Expand All @@ -37,7 +37,7 @@ export type UseUpdateErrorRecoveryPolicyOptions = UseMutationOptions<
export function useUpdateErrorRecoveryPolicy(
runId: string,
options: UseUpdateErrorRecoveryPolicyOptions = {}
): UseUpdateErrorRecoveryPolicyResponse {
): UseErrorRecoveryPolicyResponse {
const host = useHost()

const mutation = useMutation<
Expand Down
24 changes: 16 additions & 8 deletions robot-server/robot_server/runs/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
_run_orchestrator_store_accessor = AppStateAccessor[RunOrchestratorStore](
"run_orchestrator_store"
)
_run_data_manager_accessor = AppStateAccessor[RunDataManager]("run_data_manager")
_light_control_accessor = AppStateAccessor[LightController]("light_controller")


Expand Down Expand Up @@ -154,6 +155,7 @@ async def get_is_okay_to_create_maintenance_run(


async def get_run_data_manager(
app_state: Annotated[AppState, Depends(get_app_state)],
task_runner: Annotated[TaskRunner, Depends(get_task_runner)],
run_orchestrator_store: Annotated[
RunOrchestratorStore, Depends(get_run_orchestrator_store)
Expand All @@ -164,14 +166,20 @@ async def get_run_data_manager(
ErrorRecoverySettingStore, Depends(get_error_recovery_setting_store)
],
) -> RunDataManager:
"""Get a run data manager to keep track of current/historical run data."""
return RunDataManager(
run_orchestrator_store=run_orchestrator_store,
run_store=run_store,
error_recovery_setting_store=error_recovery_setting_store,
task_runner=task_runner,
runs_publisher=runs_publisher,
)
"""Get a singleton run data manager to keep track of current/historical run data."""
run_data_manager = _run_data_manager_accessor.get_from(app_state)

if run_data_manager is None:
run_data_manager = RunDataManager(
run_orchestrator_store=run_orchestrator_store,
run_store=run_store,
error_recovery_setting_store=error_recovery_setting_store,
task_runner=task_runner,
runs_publisher=runs_publisher,
)
_run_data_manager_accessor.set_on(app_state, run_data_manager)

return run_data_manager


async def get_run_auto_deleter(
Expand Down
2 changes: 2 additions & 0 deletions robot-server/robot_server/runs/router/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
from .commands_router import commands_router
from .actions_router import actions_router
from .labware_router import labware_router
from .error_recovery_policy_router import error_recovery_policy_router

runs_router = APIRouter()

runs_router.include_router(base_router)
runs_router.include_router(commands_router)
runs_router.include_router(actions_router)
runs_router.include_router(labware_router)
runs_router.include_router(error_recovery_policy_router)

__all__ = ["runs_router"]
42 changes: 0 additions & 42 deletions robot-server/robot_server/runs/router/base_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
get_run_auto_deleter,
get_quick_transfer_run_auto_deleter,
)
from ..error_recovery_models import ErrorRecoveryPolicy

from robot_server.deck_configuration.fastapi_dependencies import (
get_deck_configuration_store,
Expand Down Expand Up @@ -439,47 +438,6 @@ async def update_run(
)


@PydanticResponse.wrap_route(
base_router.put,
path="/runs/{runId}/errorRecoveryPolicy",
summary="Set a run's error recovery policy",
description=dedent(
"""
Update how to handle different kinds of command failures.
For this to have any effect, error recovery must also be enabled globally.
See `PATCH /errorRecovery/settings`.
"""
),
responses={
status.HTTP_200_OK: {"model": SimpleEmptyBody},
status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]},
},
)
async def put_error_recovery_policy(
runId: str,
request_body: RequestModel[ErrorRecoveryPolicy],
run_data_manager: Annotated[RunDataManager, Depends(get_run_data_manager)],
) -> PydanticResponse[SimpleEmptyBody]:
"""Create run polices.
Arguments:
runId: Run ID pulled from URL.
request_body: Request body with run policies data.
run_data_manager: Current and historical run data management.
"""
rules = request_body.data.policyRules
try:
run_data_manager.set_error_recovery_rules(run_id=runId, rules=rules)
except RunNotCurrentError as e:
raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT) from e

return await PydanticResponse.create(
content=SimpleEmptyBody.construct(),
status_code=status.HTTP_200_OK,
)


@PydanticResponse.wrap_route(
base_router.get,
path="/runs/{runId}/commandErrors",
Expand Down
Loading

0 comments on commit 7e6c8ee

Please sign in to comment.