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): ODD add error codes to protocol failed modal #13158

Merged
merged 4 commits into from
Jul 27, 2023
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
3 changes: 3 additions & 0 deletions api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ export interface CommandData {
export interface RunError {
id: string
errorType: string
errorInfo: { [key: string]: string }
wrappedErrors: RunError[]
errorCode: string
createdAt: string
detail: string
}
1 change: 1 addition & 0 deletions app/src/assets/localization/en/run_details.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"end_step_time": "End",
"end": "End",
"error_type": "Error: {{errorType}}",
"error_info": "Error {{errorCode}}: {{errorType}}",
"failed_step": "Failed step",
"ignore_stored_data": "Ignore stored data",
"labware_offset_data": "labware offset data",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { useHistory } from 'react-router-dom'
import isEmpty from 'lodash/isEmpty'
import { css } from 'styled-components'

import {
Expand All @@ -20,13 +21,7 @@
import { Modal } from '../../../molecules/Modal'

import type { ModalHeaderBaseProps } from '../../../molecules/Modal/types'

interface RunError {
id: string
errorType: string
createdAt: string
detail: string
}
import type { RunError } from '@opentrons/api-client'
Copy link
Contributor

Choose a reason for hiding this comment

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

We can put this line before ModalHeaderBaseProps

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah sorry i think it was after a weird merge, should be fixed now


interface RunFailedModalProps {
runId: string
Expand All @@ -44,11 +39,13 @@
const { stopRun } = useStopRunMutation()
const [isCanceling, setIsCanceling] = React.useState(false)

if (errors == null) return null
if (errors == null || errors.length === 0) return null
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to keep errors == null ?
errors type is RunError[] which errors would not get null or undefined?

Copy link
Member Author

Choose a reason for hiding this comment

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

right now errors are optional:

interface RunFailedModalProps {
  runId: string
  setShowRunFailedModal: (showRunFailedModal: boolean) => void
  errors?: RunError[]
}

because of what we get back from react query, so this is just to make type checking happy. if we want to get rid of this check then inside of RunSummary before we render RunFailedModal we have to make sure runRecord?.data.error is not null

const modalHeader: ModalHeaderBaseProps = {
title: t('run_failed_modal_title'),
}

const highestPriorityError = getHighestPriorityError(errors)

const handleClose = (): void => {
setIsCanceling(true)
setShowRunFailedModal(false)
Expand Down Expand Up @@ -76,8 +73,9 @@
alignItems={ALIGN_FLEX_START}
>
<StyledText as="p" fontWeight={TYPOGRAPHY.fontWeightBold}>
{t('error_type', {
errorType: errors[0].errorType,
{t('error_info', {
errorType: highestPriorityError.errorType,
errorCode: highestPriorityError.errorCode,
})}
</StyledText>
<Flex
Expand All @@ -89,16 +87,15 @@
borderRadius={BORDERS.borderRadiusSize3}
padding={`${SPACING.spacing16} ${SPACING.spacing20}`}
>
<Flex css={SCROLL_BAR_STYLE}>
{errors?.map(error => (
<StyledText
as="p"
key={error.id}
textAlign={TYPOGRAPHY.textAlignLeft}
>
{error.detail}
<Flex flexDirection={DIRECTION_COLUMN} css={SCROLL_BAR_STYLE}>
<StyledText as="p" textAlign={TYPOGRAPHY.textAlignLeft}>
{highestPriorityError.detail}
</StyledText>
{!isEmpty(highestPriorityError.errorInfo) && (
<StyledText as="p" textAlign={TYPOGRAPHY.textAlignLeft}>
{JSON.stringify(highestPriorityError.errorInfo)}
</StyledText>
))}
)}
</Flex>
</Flex>
<StyledText as="p" textAlign={TYPOGRAPHY.textAlignLeft}>
Expand Down Expand Up @@ -135,3 +132,63 @@
border-radius: 11px;
}
`

const _getHighestPriorityError = (error: RunError): RunError => {
Copy link
Member Author

Choose a reason for hiding this comment

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

gonna move all of these utils into a shared place in a follow up PR since ill need to use the public function for the desktop app

if (error.wrappedErrors.length === 0) {
return error
}

let highestPriorityError = error

error.wrappedErrors.forEach(wrappedError => {
const e = _getHighestPriorityError(wrappedError)
const isHigherPriority = _getIsHigherPriority(
e.errorCode,
highestPriorityError.errorCode
)
if (isHigherPriority) {
highestPriorityError = e
}
})
return highestPriorityError
}

/**
* returns true if the first error code is higher priority than the second, false otherwise
*/
const _getIsHigherPriority = (
errorCode1: string,
errorCode2: string
): boolean => {
const errorNumber1 = Number(errorCode1)
const errorNumber2 = Number(errorCode2)

const isSameCategory =
Math.floor(errorNumber1 / 1000) === Math.floor(errorNumber2 / 1000)
const isCode1GenericError = errorNumber1 % 1000 === 0

let isHigherPriority = null

if (
(isSameCategory && !isCode1GenericError) ||
(!isSameCategory && errorNumber1 < errorNumber2)
) {
isHigherPriority = true
} else {
isHigherPriority = false
}

return isHigherPriority
}

export const getHighestPriorityError = (errors: RunError[]): RunError => {
const highestFirstWrappedError = _getHighestPriorityError(errors[0])
return [highestFirstWrappedError, ...errors.slice(1)].reduce((acc, val) => {
const e = _getHighestPriorityError(val)
const isHigherPriority = _getIsHigherPriority(e.errorCode, acc.errorCode)
if (isHigherPriority) {
return e

Check warning on line 190 in app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx

View check run for this annotation

Codecov / codecov/patch

app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx#L190

Added line #L190 was not covered by tests
}
return acc
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,53 @@ const mockPush = jest.fn()
const mockErrors = [
{
id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba',
errorType: 'ExceptionInProtocolError',
errorType: 'generalError',
createdAt: '2023-04-09T21:41:51.333171+00:00',
detail:
'ProtocolEngineError [line 16]: ModuleNotAttachedError: No available',
detail: 'Error with code 4000 (lowest priority)',
errorInfo: {},
errorCode: '4000',
wrappedErrors: [
{
id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba',
errorType: 'roboticsInteractionError',
createdAt: '2023-04-09T21:41:51.333171+00:00',
detail: 'Error with code 3000 (second lowest priortiy)',
errorInfo: {},
errorCode: '3000',
wrappedErrors: [],
},
{
id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba',
errorType: 'roboticsControlError',
createdAt: '2023-04-09T21:41:51.333171+00:00',
detail: 'Error with code 2000 (second highest priority)',
errorInfo: {},
errorCode: '2000',
wrappedErrors: [
{
id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba',
errorType: 'hardwareCommunicationError',
createdAt: '2023-04-09T21:41:51.333171+00:00',
detail: 'Error with code 1000 (highest priority)',
errorInfo: {},
errorCode: '1000',
wrappedErrors: [],
},
],
},
],
},
{
id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba',
errorType: 'roboticsInteractionError',
createdAt: '2023-04-09T21:41:51.333171+00:00',
detail: 'Error with code 2001 (second highest priortiy)',
errorInfo: {},
errorCode: '2001',
wrappedErrors: [],
},
]

let mockStopRun: jest.Mock

jest.mock('react-router-dom', () => {
Expand Down Expand Up @@ -60,12 +101,11 @@ describe('RunFailedModal', () => {
mockUseStopRunMutation.mockReturnValue({ stopRun: mockStopRun } as any)
})

it('should render text and button', () => {
it('should render the highest priority error', () => {
const [{ getByText }] = render(props)
getByText('Run failed')
getByText(
'ProtocolEngineError [line 16]: ModuleNotAttachedError: No available'
)
getByText('Error 1000: hardwareCommunicationError')
getByText('Error with code 1000 (highest priority)')
getByText(
'Download the run logs from the Opentrons App and send it to [email protected] for assistance.'
)
Expand All @@ -79,7 +119,4 @@ describe('RunFailedModal', () => {
expect(mockStopRun).toHaveBeenCalled()
expect(mockPush).toHaveBeenCalledWith('/dashboard')
})
// ToDo (kj:04/12/2023) I made this test todo since we need the system update to align with the design.
// This test will be added when we can get error code and other information
it.todo('should render error code and message')
})
3 changes: 3 additions & 0 deletions app/src/organisms/RunTimeControl/__fixtures__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export const mockFailedRun: RunData = {
errorType: 'RuntimeError',
createdAt: 'noon forty-five',
detail: 'this run failed',
errorInfo: {},
wrappedErrors: [],
errorCode: '4000',
},
],
pipettes: [],
Expand Down
Loading