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

fix(api-client, react-api-client, app, robot-server): support multiple recovery policies during a run #16950

Merged
merged 14 commits into from
Nov 22, 2024
Merged
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
Loading