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

.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 @@
 export const configInitialized = (config: Config): ConfigInitializedAction => ({
   type: CONFIG_INITIALIZED,
   payload: { config },
+  meta: { shell: true },
 })
 
 // config value has been updated
@@ -128,6 +129,7 @@
 ): ConfigValueUpdatedAction => ({
   type: VALUE_UPDATED,
   payload: { path, value },
+  meta: { shell: true },
 })
 
 export const customLabwareList = (
@@ -355,7 +357,7 @@
 export const appRestart = (message: string): AppRestartAction => ({
   type: APP_RESTART,
   payload: {
     message: message,
   },
   meta: { shell: true },
 })
@@ -363,7 +365,7 @@
 export const reloadUi = (message: string): ReloadUiAction => ({
   type: RELOAD_UI,
   payload: {
     message: message,
   },
   meta: { shell: true },
 })
@@ -371,7 +373,7 @@
 export const sendLog = (message: string): SendLogAction => ({
   type: SEND_LOG,
   payload: {
     message: message,
   },
   meta: { shell: true },
 })
@@ -379,7 +381,7 @@
 export const updateBrightness = (message: string): UpdateBrightnessAction => ({
   type: UPDATE_BRIGHTNESS,
   payload: {
     message: message,
   },
   meta: { shell: true },
 })

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')