Skip to content

Commit

Permalink
refactor(app, app-shell): remove try again button and prevent errorno…
Browse files Browse the repository at this point in the history
… 22 (#13907)

Zip file robot updates ocassionally cause issues with retrying updates, especially on Windows.
Remove the retry logic and file upload progress until sorted.

Co-authored-by: Shlok Amin <[email protected]>
  • Loading branch information
mjhuff and shlokamin authored Nov 2, 2023
1 parent 7d99178 commit a779679
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 59 deletions.
2 changes: 1 addition & 1 deletion app-shell/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ function createReadStreamWithSize(
}

readStream.once('error', handleError)
readStream.on('data', onData)

function handleSuccess(): void {
resolve(readStream)
readStream.removeListener('error', handleError)
}

function handleError(error: Error): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ export function UpdateRobotSoftware({
dispatchStartRobotUpdate(robotName, files[0].path)
onUpdateStart()
}
// this is to reset the state of the file picker so users can reselect the same
// system image if the upload fails
if (inputRef.current?.value != null) {
inputRef.current.value = ''
}
}

const handleClick: React.MouseEventHandler<HTMLButtonElement> = () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { useDispatch, useSelector } from 'react-redux'
import { useDispatch } from 'react-redux'
import { css } from 'styled-components'

import {
Flex,
Icon,
Link,
NewPrimaryBtn,
NewSecondaryBtn,
JUSTIFY_FLEX_END,
ALIGN_CENTER,
COLORS,
Expand All @@ -25,13 +23,10 @@ import { FOOTER_BUTTON_STYLE } from './UpdateRobotModal'
import {
startRobotUpdate,
clearRobotUpdateSession,
getRobotSessionIsManualFile,
} from '../../../../redux/robot-update'
import { useDispatchStartRobotUpdate } from '../../../../redux/robot-update/hooks'
import { useRobotUpdateInfo } from './useRobotUpdateInfo'
import successIcon from '../../../../assets/images/icon_success.png'

import type { State } from '../../../../redux/types'
import type { SetStatusBarCreateCommand } from '@opentrons/shared-data/protocol/types/schemaV7/command/incidental'
import type { RobotUpdateSession } from '../../../../redux/robot-update/types'
import type { UpdateStep } from './useRobotUpdateInfo'
Expand All @@ -47,10 +42,6 @@ const UPDATE_TEXT_STYLE = css`
color: ${COLORS.darkGreyEnabled};
font-size: 0.8rem;
`
const TRY_RESTART_STYLE = css`
color: ${COLORS.blueEnabled};
font-size: 0.8rem;
`
const HIDDEN_CSS = css`
position: fixed;
clip: rect(1px 1px 1px 1px);
Expand All @@ -71,17 +62,10 @@ export function RobotUpdateProgressModal({
const { t } = useTranslation('device_settings')
const [showFileSelect, setShowFileSelect] = React.useState<boolean>(false)
const installFromFileRef = React.useRef<HTMLInputElement>(null)
const dispatchStartRobotUpdate = useDispatchStartRobotUpdate()
const manualFileUsedForUpdate = useSelector((state: State) =>
getRobotSessionIsManualFile(state)
)

const completeRobotUpdateHandler = (): void => {
if (closeUpdateBuildroot != null) closeUpdateBuildroot()
}
const reinstallUpdate = React.useCallback(() => {
dispatchStartRobotUpdate(robotName)
}, [robotName])

const { error } = session || { error: null }
const { updateStep, progressPercent } = useRobotUpdateInfo(session)
useStatusBarAnimation(error != null)
Expand Down Expand Up @@ -125,9 +109,6 @@ export function RobotUpdateProgressModal({
footer={
hasStoppedUpdating ? (
<RobotUpdateProgressFooter
robotName={robotName}
installRobotUpdate={dispatchStartRobotUpdate}
errorMessage={error}
closeUpdateBuildroot={completeRobotUpdateHandler}
/>
) : null
Expand All @@ -151,17 +132,7 @@ export function RobotUpdateProgressModal({
<StyledText css={UPDATE_TEXT_STYLE}>
{letUserExitUpdate && updateStep !== 'restart' ? (
<>
{t('problem_during_update')}{' '}
<Link
css={TRY_RESTART_STYLE}
onClick={
!manualFileUsedForUpdate
? reinstallUpdate
: () => setShowFileSelect(true)
}
>
{t('try_restarting_the_update')}
</Link>
{t('problem_during_update')} {t('try_restarting_the_update')}
{showFileSelect && (
<input
ref={installFromFileRef}
Expand All @@ -182,35 +153,16 @@ export function RobotUpdateProgressModal({
}

interface RobotUpdateProgressFooterProps {
robotName: string
installRobotUpdate: (robotName: string) => void
errorMessage?: string | null
closeUpdateBuildroot?: () => void
}

function RobotUpdateProgressFooter({
robotName,
installRobotUpdate,
errorMessage,
closeUpdateBuildroot,
}: RobotUpdateProgressFooterProps): JSX.Element {
const { t } = useTranslation('device_settings')
const installUpdate = React.useCallback(() => {
installRobotUpdate(robotName)
}, [robotName])

return (
<Flex alignItems={ALIGN_CENTER} justifyContent={JUSTIFY_FLEX_END}>
{errorMessage && (
<NewSecondaryBtn
onClick={installUpdate}
marginRight={SPACING.spacing8}
css={FOOTER_BUTTON_STYLE}
border="none"
>
{t('try_again')}
</NewSecondaryBtn>
)}
<NewPrimaryBtn
onClick={closeUpdateBuildroot}
marginRight={SPACING.spacing12}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ describe('DownloadUpdateModal', () => {

expect(getByText('test error')).toBeInTheDocument()
fireEvent.click(exitButton)
expect(getByText('Try again')).toBeInTheDocument()
expect(props.closeUpdateBuildroot).toHaveBeenCalled()

expect(mockUseCreateLiveCommandMutation).toBeCalledWith()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('useRobotUpdateInfo', () => {
)

expect(result.current.updateStep).toBe('install')
expect(Math.round(result.current.progressPercent)).toBe(67)
expect(Math.round(result.current.progressPercent)).toBe(75)

rerender({
...mockRobotUpdateSession,
Expand All @@ -70,15 +70,15 @@ describe('useRobotUpdateInfo', () => {
)

expect(result.current.updateStep).toBe('install')
expect(Math.round(result.current.progressPercent)).toBe(67)
expect(Math.round(result.current.progressPercent)).toBe(75)

rerender({
...mockRobotUpdateSession,
error: 'Something went wrong',
})

expect(result.current.updateStep).toBe('error')
expect(Math.round(result.current.progressPercent)).toBe(67)
expect(Math.round(result.current.progressPercent)).toBe(75)
})

it('should calculate correct progressPercent when the update is not manual', () => {
Expand All @@ -94,7 +94,7 @@ describe('useRobotUpdateInfo', () => {
})

expect(result.current.updateStep).toBe('install')
expect(Math.round(result.current.progressPercent)).toBe(67)
expect(Math.round(result.current.progressPercent)).toBe(75)
})

it('should ignore progressPercent reported by a step marked as ignored', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ function useFindProgressPercentFrom(

const stepAndStage = `${sessionStep}-${sessionStage}`
// Ignored because 0-100 is too fast to be worth recording.
const IGNORED_STEPS_AND_STAGES = ['processFile-awaiting-file']
const IGNORED_STEPS_AND_STAGES = [
'processFile-awaiting-file',
'uploadFile-awaiting-file',
]
// Each stepAndStage is an equal fraction of the total steps.
const TOTAL_STEPS_WITH_PROGRESS = 3
const TOTAL_STEPS_WITH_PROGRESS = 2

const isNewStateWithProgress =
prevSeenUpdateStep.current !== stepAndStage &&
Expand Down

0 comments on commit a779679

Please sign in to comment.