refactor(app-shell-odd): overhaul odd system update (#16486)

Flex robots are capable of updating their own software from the Internet
or from USB sticks containing update files. This functionality is
handled by the ODD's node side in `app-shell-odd`. It was implemented
early in the flex development life cycle before we'd gotten a strong
understanding of how everything would work, and was therefore a little
scattershot and not really conformant to the way we now think about and
structure our code. It also had some unexpected behaviors and was
generally a little buggy.

This PR refactors `app-shell-odd`'s system update mechanisms, ideally
fixing said bugs.

## non-goals
### changing the frontend or the redux interaction patterns
this was already a pretty big pr, so I didn't touch these. They probably
should be touched, though; sourcing updates from web or from usb is a
different thing from sourcing updates from web or from the user's file
browser, which is what the UI side and the redux messages are designed
to implement, and the concepts don't map terribly well.

The redux message flow and semantics should not change in this PR, and
this PR doesn't touch any app code

### changing the precedence or user-visible functionality of system
updates
We still have this idea that
- Updates should be presented when an available update is different than
the current system version
- Updates from USB sticks have higher precedence than updates from the
internet, even if the update from the internet has a higher version than
the update from the USB stick
- If multiple updates from sources of the same precedence are available
(i.e., a USB stick with multiple update files is connected) then the
update with the highest version takes precedence

Changing this would require changing the frontend to have more than a
"press a button to start an available update" type of flow, and require
changing the redux messages.

## goals
### make the code easier to reason about
The code was split sort of weirdly between a single ts file and a module
of ts files; code to handle USB updates was scattered around the code to
handle internet updates; it was generally a tough assignment to figure
out what would happen when user actions are taken or redux messages are
sent. Make it easier to understand
### make the code more robust
The code didn't really uniformly handle errors or handle errors in the
same way in multiple places; this is pretty much guaranteed because the
code didn't really have tests.

## changes
- move all the system update code into `system-update`
- separate the code strongly by layer. the layer that handles the redux
messages is in `system-update/handler.ts`, and it imports functions from
lower levels in the `from-usb` and `from-web` modules.
- separate the code strongly by source. all code dealing with updates
from the web is in the `system-updates/from-web` moduels; the code
dealing with updates from usb is in `system-updates/from-usb`. Each
module presents a closure with a well-defined interface that the handler
can use.
- remove global mutable state in favor of closed-over mutable state for
better testability
- add tests, for everything (this is where the size of the diff is from)

## review requests
- this is a pr that is mostly motivated by readability and robustness,
so opinions on how readable and robust the code is are very welcome and
needed
- is the behavior correct from the description above?

## reviewing
This ended up being a pretty big PR, sorry! I think a good way to
approach looking at is to structure into the mostly-separate changes,
which only affect each other implicitly:
- the new cancellation mechanics in HTTP as its own thing
- the action distribution changes in `main`
- the `app.getPath()` changes in `main` and in `config`
- the mechanics of `system-update/handler` first, then the two update
providers underneath; the logic here is separated into "conveying the
status of updates" in `handler.ts` and "getting updates" in the
providers
- the usb device sensing changes, which handle mass storage devices that
are like `/dev/sda` but mounted at `/media/VOLUMENAME-sda` and fix
excessive timeouts in the recursive enumeration

## testing
this needs to be tested on a flex (conveniently, the OT-2 doesn't run
this code). things that should be tested are
- [x] when the robot is connected to the internet on an update channel
that has an available update that is higher than the current robot
version, does the system download, present, and apply the update
- [x] when the robot is connected to the internet on an update channel
that has an available update that is lower than or the same as the
current robot version, does the system download but not present the
update
- [x] when the robot is connected to the internet on an update channel
that has an available update, then disconnected after downloading the
available update, does the system present and apply the update
- [x] when the robot is disconnected from the internet while downloading
an available update, does the system not present the update
- [x] when you change the update channel, does the presented update get
properly cleared and re-queried, including
- [x] changing from a channel with an update to a channel with a
different update
- [x] changing from a channel with an update to a channel without an
update
- [x] changing from a channel without an update to a channel with an
update
- [x] when you plug in a usb stick with an update, it gets detected,
presented, and applied
   - [x] including when an update is available from the internet
- [x] including when the internet update is of a higher version than the
usb update
   - [x] including when there are multiple updates on the usb stick
- [x] including when the update on the usb stick is a lower version than
the current version of the robot

Closes RQA-3060

.github/workflows/app-test-build-deploy.yaml

@@ -97,7 +97,11 @@ jobs:
     strategy:
       matrix:
         os: ['windows-2022', 'ubuntu-22.04', 'macos-latest']
-    name: 'opentrons app backend unit tests on ${{matrix.os}}'
+        shell: ['app-shell', 'app-shell-odd', 'discovery-client']
+        exclude:
+          - os: 'windows-2022'
+            shell: 'app-shell-odd'
+    name: 'opentrons ${{matrix.shell}} unit tests on ${{matrix.os}}'
     timeout-minutes: 60
     runs-on: ${{ matrix.os }}
     steps:
@@ -144,7 +148,7 @@ jobs:
           yarn config set cache-folder ${{ github.workspace }}/.yarn-cache
           make setup-js
       - name: 'test native(er) packages'
-        run: make test-js-internal tests="app-shell/src app-shell-odd/src discovery-client/src" cov_opts="--coverage=true"
+        run: make test-js-internal tests="${{}matrix.shell}/src" cov_opts="--coverage=true"
       - name: 'Upload coverage report'
         uses: 'codecov/codecov-action@v3'
         with:

app-shell-odd/src/__tests__/http.test.ts

@@ -9,6 +9,7 @@ import type { Request, Response } from 'node-fetch'
 
 vi.mock('../config')
 vi.mock('node-fetch')
+vi.mock('../log')
 
 describe('app-shell main http module', () => {
   beforeEach(() => {

app-shell-odd/src/actions.ts

@@ -119,6 +119,7 @@ import type {
 export const configInitialized = (config: Config): ConfigInitializedAction => ({
   type: CONFIG_INITIALIZED,
   payload: { config },
+  meta: { shell: true },
 })
 
 // config value has been updated
@@ -128,6 +129,7 @@ export const configValueUpdated = (
 ): ConfigValueUpdatedAction => ({
   type: VALUE_UPDATED,
   payload: { path, value },
+  meta: { shell: true },
 })
 
 export const customLabwareList = (

app-shell-odd/src/config/index.ts

@@ -5,14 +5,14 @@ import get from 'lodash/get'
 import forEach from 'lodash/forEach'
 import mergeOptions from 'merge-options'
 import yargsParser from 'yargs-parser'
-
 import { UI_INITIALIZED } from '../constants'
 import * as Cfg from '../constants'
 import { configInitialized, configValueUpdated } from '../actions'
 import systemd from '../systemd'
 import { createLogger } from '../log'
 import { DEFAULTS_V12, migrate } from './migrate'
 import { shouldUpdate, getNextValue } from './update'
+import { setUserDataPath } from '../early'
 
 import type {
   ConfigV12,
@@ -24,8 +24,6 @@ import type { Config, Overrides } from './types'
 
 export * from './types'
 
-export const ODD_DIR = '/data/ODD'
-
 // make sure all arguments are included in production
 const argv = process.argv0.endsWith('defaultApp')
   ? process.argv.slice(2)
@@ -48,8 +46,7 @@ const store = (): Store => {
     // perform store migration if loading for the first time
     _store = (new Store({
       defaults: DEFAULTS_V12,
-      // dont overwrite config dir if in dev mode because it causes issues
-      ...(process.env.NODE_ENV === 'production' && { cwd: ODD_DIR }),
+      cwd: setUserDataPath(),
     }) as unknown) as Store<Config>
     _store.store = migrate((_store.store as unknown) as ConfigV12)
   }
@@ -66,7 +63,14 @@ const log = (): Logger => _log ?? (_log = createLogger('config'))
 export function registerConfig(dispatch: Dispatch): (action: Action) => void {
   return function handleIncomingAction(action: Action) {
     if (action.type === UI_INITIALIZED) {
+      log().info('initializing configuration')
       dispatch(configInitialized(getFullConfig()))
+      log().info(
+        `flow route: ${
+          getConfig('onDeviceDisplaySettings').unfinishedUnboxingFlowRoute
+        }`
+      )
+      log().info('configuration initialized')
     } else if (
       action.type === Cfg.UPDATE_VALUE ||
       action.type === Cfg.RESET_VALUE ||
@@ -120,8 +124,8 @@ export function getOverrides(path?: string): unknown {
   return path != null ? get(overrides(), path) : overrides()
 }
 
-export function getConfig<P extends keyof Config>(path: P): Config[P]
 export function getConfig(): Config
+export function getConfig<P extends keyof Config>(path: P): Config[P]
 export function getConfig(path?: any): any {
   const result = store().get(path)
   const over = getOverrides(path as string | undefined)

app-shell-odd/src/constants.ts

@@ -257,3 +257,5 @@ export const FAILURE_STATUSES = {
 } as const
 
 export const SEND_FILE_PATHS: 'shell:SEND_FILE_PATHS' = 'shell:SEND_FILE_PATHS'
+
+export const ODD_DATA_DIR = '/data/ODD'

app-shell-odd/src/early.ts

@@ -0,0 +1,22 @@
+// things intended to execute early in app-shell initialization
+// do as little as possible in this file and do none of it at import time
+
+import { app } from 'electron'
+import { ODD_DATA_DIR } from './constants'
+
+let path: string
+
+export const setUserDataPath = (): string => {
+  if (path == null) {
+    console.log(
+      `node env is ${process.env.NODE_ENV}, path is ${app.getPath('userData')}`
+    )
+    if (process.env.NODE_ENV === 'production') {
+      console.log(`setting app path to ${ODD_DATA_DIR}`)
+      app.setPath('userData', ODD_DATA_DIR)
+    }
+    path = app.getPath('userData')
+    console.log(`app path becomes ${app.getPath('userData')}`)
+  }
+  return app.getPath('userData')
+}

app-shell-odd/src/http.ts

@@ -7,17 +7,30 @@ import FormData from 'form-data'
 import { Transform } from 'stream'
 
 import { HTTP_API_VERSION } from './constants'
+import { createLogger } from './log'
 
 import type { Readable } from 'stream'
 import type { Request, RequestInit, Response } from 'node-fetch'
 
+const log = createLogger('http')
+
 type RequestInput = Request | string
 
 export interface DownloadProgress {
   downloaded: number
   size: number | null
 }
 
+export class LocalAbortError extends Error {
+  declare readonly name: 'LocalAbortError'
+  declare readonly type: 'aborted'
+  constructor(message: string) {
+    super(message)
+    this.name = 'LocalAbortError'
+    this.type = 'aborted'
+  }
+}
+
 export function fetch(
   input: RequestInput,
   init?: RequestInit
@@ -35,21 +48,29 @@ export function fetch(
   })
 }
 
-export function fetchJson<Body>(input: RequestInput): Promise<Body> {
-  return fetch(input).then(response => response.json())
+export function fetchJson<Body>(
+  input: RequestInput,
+  init?: RequestInit
+): Promise<Body> {
+  return fetch(input, init).then(response => response.json())
+}
+
+export function fetchText(input: Request, init?: RequestInit): Promise<string> {
+  return fetch(input, init).then(response => response.text())
 }
 
-export function fetchText(input: Request): Promise<string> {
-  return fetch(input).then(response => response.text())
+export interface FetchToFileOptions {
+  onProgress: (progress: DownloadProgress) => unknown
+  signal: AbortSignal
 }
 
 // TODO(mc, 2019-07-02): break this function up and test its components
 export function fetchToFile(
   input: RequestInput,
   destination: string,
-  options?: Partial<{ onProgress: (progress: DownloadProgress) => unknown }>
+  options?: Partial<FetchToFileOptions>
 ): Promise<string> {
-  return fetch(input).then(response => {
+  return fetch(input, { signal: options?.signal }).then(response => {
     let downloaded = 0
     const size = Number(response.headers.get('Content-Length')) || null
 
@@ -75,13 +96,26 @@ export function fetchToFile(
       // pump calls stream.pipe, handles teardown if streams error, and calls
       // its callbacks when the streams are done
       pump(inputStream, progressReader, outputStream, error => {
-        // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-        if (error) {
+        const handleError = (problem: Error): void => {
           // if we error out, delete the temp dir to clean up
-          return remove(destination).then(() => {
+          log.error(`Aborting fetchToFile: ${problem.name}: ${problem.message}`)
+          remove(destination).then(() => {
             reject(error)
           })
         }
+        const listener = (): void => {
+          handleError(
+            new LocalAbortError(
+              (options?.signal?.reason as string | null) ?? 'aborted'
+            )
+          )
+        }
+        options?.signal?.addEventListener('abort', listener, { once: true })
+        // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
+        if (error) {
+          handleError(error)
+        }
+        options?.signal?.removeEventListener('abort', listener, {})
         resolve(destination)
       })
     })

app-shell-odd/src/log.ts

@@ -4,13 +4,13 @@ import path from 'path'
 import dateFormat from 'dateformat'
 import winston from 'winston'
 
+import { setUserDataPath } from './early'
 import { getConfig } from './config'
 
 import type Transport from 'winston-transport'
 import type { Config } from './config'
 
-const ODD_DIR = '/data/ODD'
-const LOG_DIR = path.join(ODD_DIR, 'logs')
+const LOG_DIR = path.join(setUserDataPath(), 'logs')
 const ERROR_LOG = path.join(LOG_DIR, 'error.log')
 const COMBINED_LOG = path.join(LOG_DIR, 'combined.log')