Merge pull request #2236 from GMOD/assembly_loading_error

Detect assembly loading error and encapsulate error instead of failing at app level

config/jest/console.js

@@ -1,4 +1,5 @@
 const originalWarn = console.warn
+const originalError = console.error
 
 // this is here to silence a warning related to @material-ui/data-grid
 // not precisely sure why it warns but this error is silenced during test
@@ -8,3 +9,10 @@ jest.spyOn(console, 'warn').mockImplementation((...args) => {
   }
   return originalWarn.call(console, ...args)
 })
+
+jest.spyOn(console, 'error').mockImplementation((...args) => {
+  if (String(args[0]).includes('volvox.2bit_404')) {
+    return undefined
+  }
+  return originalError.call(console, ...args)
+})

packages/core/assemblyManager/assembly.ts

@@ -1,7 +1,7 @@
 import jsonStableStringify from 'json-stable-stringify'
 import { getParent, IAnyType, types, Instance } from 'mobx-state-tree'
 import AbortablePromiseCache from 'abortable-promise-cache'
-import { readConfObject } from '../configuration'
+import { getConf } from '../configuration'
 import {
   BaseRefNameAliasAdapter,
   RegionsAdapter,
@@ -120,6 +120,13 @@ interface CacheData {
   sessionId: string
   options: BaseOptions
 }
+
+export interface BasicRegion {
+  start: number
+  end: number
+  refName: string
+  assemblyName: string
+}
 export default function assemblyFactory(
   assemblyConfigType: IAnyType,
   pluginManager: PluginManager,
@@ -146,26 +153,20 @@ export default function assemblyFactory(
       configuration: types.safeReference(assemblyConfigType),
     })
     .volatile(() => ({
-      regions: undefined as
-        | {
-            start: number
-            end: number
-            refName: string
-            assemblyName: string
-          }[]
-        | undefined,
+      error: undefined as Error | undefined,
+      regions: undefined as BasicRegion[] | undefined,
       refNameAliases: undefined as { [key: string]: string } | undefined,
     }))
     .views(self => ({
       get initialized() {
         return Boolean(self.refNameAliases)
       },
       get name(): string {
-        return readConfObject(self.configuration, 'name')
+        return getConf(self, 'name')
       },
 
       get aliases(): string[] {
-        return readConfObject(self.configuration, 'aliases')
+        return getConf(self, 'aliases')
       },
 
       hasName(name: string) {
@@ -179,19 +180,15 @@ export default function assemblyFactory(
         return self.regions && self.regions.map(region => region.refName)
       },
       get allRefNames() {
-        if (!self.refNameAliases) {
-          return undefined
-        }
-        return Object.keys(self.refNameAliases)
+        return !self.refNameAliases
+          ? undefined
+          : Object.keys(self.refNameAliases)
       },
       get rpcManager() {
         return getParent(self, 2).rpcManager
       },
       get refNameColors() {
-        const colors = readConfObject(
-          self.configuration,
-          'refNameColors',
-        ) as string[]
+        const colors: string[] = getConf(self, 'refNameColors')
         if (colors.length === 0) {
           return refNameColors
         }
@@ -237,7 +234,7 @@ export default function assemblyFactory(
       },
       setError(error: Error) {
         if (!getParent(self, 3).isAssemblyEditing) {
-          getParent(self, 3).setError(error)
+          self.error = error
         }
       },
       setRegions(regions: Region[]) {

packages/core/assemblyManager/assemblyManager.ts

@@ -28,6 +28,10 @@ export default function assemblyManagerFactory(
         return self.assemblies.find(assembly => assembly.hasName(assemblyName))
       },
 
+      get assemblyNamesList() {
+        return this.assemblyList.map(asm => asm.name)
+      },
+
       get assemblyList() {
         // name is the explicit identifier and can be accessed without getConf,
         // hence the union with {name:string}
@@ -67,7 +71,14 @@ export default function assemblyManagerFactory(
         }
         const assembly = self.get(assemblyName)
         if (assembly) {
-          await when(() => Boolean(assembly.regions && assembly.refNameAliases))
+          await when(
+            () =>
+              Boolean(assembly.regions && assembly.refNameAliases) ||
+              !!assembly.error,
+          )
+          if (assembly.error) {
+            throw assembly.error
+          }
           return assembly
         }
         return undefined

plugins/circular-view/src/CircularView/components/CircularView.js

@@ -1,4 +1,4 @@
-import React, { useState } from 'react'
+import React from 'react'
 import { observer } from 'mobx-react'
 
 import ZoomOut from '@material-ui/icons/ZoomOut'
@@ -10,21 +10,14 @@ import LockOpen from '@material-ui/icons/LockOpen'
 import { TrackSelector as TrackSelectorIcon } from '@jbrowse/core/ui/Icons'
 
 // material-ui stuff
-import {
-  Button,
-  Container,
-  Grid,
-  IconButton,
-  MenuItem,
-  TextField,
-  makeStyles,
-} from '@material-ui/core'
+import { IconButton, makeStyles } from '@material-ui/core'
 
 import { grey } from '@material-ui/core/colors'
 
 import { ResizeHandle } from '@jbrowse/core/ui'
-import { assembleLocString, getSession } from '@jbrowse/core/util'
+import { assembleLocString } from '@jbrowse/core/util'
 import Ruler from './Ruler'
+import ImportForm from './ImportForm'
 
 const dragHandleHeight = 3
 
@@ -172,63 +165,6 @@ const Controls = observer(({ model, showingFigure }) => {
   )
 })
 
-const ImportForm = observer(({ model }) => {
-  const classes = useStyles()
-  const [selectedAssemblyIdx, setSelectedAssemblyIdx] = useState(0)
-  const { assemblyNames, assemblyManager } = getSession(model)
-  const assemblyError = assemblyNames.length ? '' : 'No configured assemblies'
-  const assembly = assemblyManager.get(assemblyNames[selectedAssemblyIdx])
-  const regions = assembly?.regions || []
-
-  function onAssemblyChange(event) {
-    setSelectedAssemblyIdx(Number(event.target.value))
-  }
-
-  function onOpenClick() {
-    model.setDisplayedRegions(regions)
-  }
-
-  return (
-    <>
-      <Container className={classes.importFormContainer}>
-        <Grid container spacing={1} justifyContent="center" alignItems="center">
-          <Grid item>
-            <TextField
-              select
-              value={
-                assemblyNames[selectedAssemblyIdx] && !assemblyError
-                  ? selectedAssemblyIdx
-                  : ''
-              }
-              onChange={onAssemblyChange}
-              helperText={assemblyError || 'Select assembly to view'}
-              error={!!assemblyError}
-              disabled={!!assemblyError}
-              margin="normal"
-            >
-              {assemblyNames.map((name, idx) => (
-                <MenuItem key={name} value={idx}>
-                  {name}
-                </MenuItem>
-              ))}
-            </TextField>
-          </Grid>
-          <Grid item>
-            <Button
-              disabled={!(regions && regions.length)}
-              onClick={onOpenClick}
-              variant="contained"
-              color="primary"
-            >
-              {regions.length ? 'Open' : 'Loading…'}
-            </Button>
-          </Grid>
-        </Grid>
-      </Container>
-    </>
-  )
-})
-
 const CircularView = observer(({ model }) => {
   const classes = useStyles()
   const initialized =

plugins/circular-view/src/CircularView/components/ImportForm.tsx

@@ -0,0 +1,87 @@
+import React, { useState } from 'react'
+import { observer } from 'mobx-react'
+import { getSession } from '@jbrowse/core/util'
+
+// material-ui stuff
+import {
+  Button,
+  Container,
+  Grid,
+  Typography,
+  makeStyles,
+} from '@material-ui/core'
+import AssemblySelector from '@jbrowse/core/ui/AssemblySelector'
+
+const useStyles = makeStyles(theme => ({
+  importFormContainer: {
+    marginBottom: theme.spacing(4),
+  },
+}))
+
+const ErrorDisplay = observer(({ error }: { error?: Error | string }) => {
+  return (
+    <Typography variant="h6" color="error">
+      {`${error}`}
+    </Typography>
+  )
+})
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const ImportForm = observer(({ model }: { model: any }) => {
+  const classes = useStyles()
+  const session = getSession(model)
+  const { error: modelError } = model
+  const { assemblyNames, assemblyManager } = session
+  const [selectedAsm, setSelectedAsm] = useState(assemblyNames[0])
+  const [error, setError] = useState<Error | undefined>(modelError)
+  const assembly = assemblyManager.get(selectedAsm)
+  const assemblyError = assemblyNames.length
+    ? assembly?.error
+    : 'No configured assemblies'
+  const regions = assembly?.regions || []
+  const err = assemblyError || error
+
+  return (
+    <>
+      <Container className={classes.importFormContainer}>
+        {err ? (
+          <Grid
+            container
+            spacing={1}
+            justifyContent="center"
+            alignItems="center"
+          >
+            <Grid item>
+              <ErrorDisplay error={err} />
+            </Grid>
+          </Grid>
+        ) : null}
+        <Grid container spacing={1} justifyContent="center" alignItems="center">
+          <Grid item>
+            <AssemblySelector
+              onChange={val => {
+                setError(undefined)
+                setSelectedAsm(val)
+              }}
+              session={session}
+              selected={selectedAsm}
+            />
+          </Grid>
+
+          <Grid item>
+            <Button
+              disabled={!(regions && regions.length)}
+              onClick={() => model.setDisplayedRegions(regions)}
+              variant="contained"
+              color="primary"
+            >
+              {regions.length ? 'Open' : 'Loading…'}
+            </Button>
+          </Grid>
+        </Grid>
+      </Container>
+    </>
+  )
+})
+
+export default ImportForm