-
Notifications
You must be signed in to change notification settings - Fork 178
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 { | ||
|
@@ -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' | ||
|
||
interface RunFailedModalProps { | ||
runId: string | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right now errors are optional:
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 |
||
const modalHeader: ModalHeaderBaseProps = { | ||
title: t('run_failed_modal_title'), | ||
} | ||
|
||
const highestPriorityError = getHighestPriorityError(errors) | ||
|
||
const handleClose = (): void => { | ||
setIsCanceling(true) | ||
setShowRunFailedModal(false) | ||
|
@@ -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 | ||
|
@@ -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}> | ||
|
@@ -135,3 +132,63 @@ | |
border-radius: 11px; | ||
} | ||
` | ||
|
||
const _getHighestPriorityError = (error: RunError): RunError => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
return acc | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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', () => { | ||
|
@@ -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.' | ||
) | ||
|
@@ -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') | ||
}) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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