Show CLI errors generated by exp show

Author: mattseddon

extension/src/experiments/index.ts

@@ -407,7 +407,10 @@ export class Experiments extends BaseRepository<TableData> {
   }
 
   public getWorkspaceAndCommits() {
-    if (!this.columns.hasNonDefaultColumns()) {
+    if (
+      !this.experiments.getCliError() &&
+      !this.columns.hasNonDefaultColumns()
+    ) {
       return []
     }
 
@@ -499,6 +502,10 @@ export class Experiments extends BaseRepository<TableData> {
     return this.experiments.hasRunningWorkspaceExperiment()
   }
 
+  public getCliError() {
+    return this.experiments.getCliError()
+  }
+
   public getFirstThreeColumnOrder() {
     return this.columns.getFirstThreeColumnOrder()
   }

extension/src/experiments/model/collect.ts

@@ -47,6 +47,7 @@ export type ExperimentItem = {
 }
 
 type ExperimentsAccumulator = {
+  cliError: string | undefined
   commits: Experiment[]
   experimentsByCommit: Map<string, Experiment[]>
   hasCheckpoints: boolean
@@ -330,12 +331,22 @@ const hasCheckpoints = (data: ExpShowOutput) => {
   return !!workspace.data.meta.has_checkpoints
 }
 
+const collectCliError = (
+  acc: ExperimentsAccumulator,
+  expShow: ExpShowOutput
+) => {
+  if (expShow.length === 1 && acc.workspace.error) {
+    acc.cliError = acc.workspace.error
+  }
+}
+
 export const collectExperiments = (
   expShow: ExpShowOutput,
   gitLog: string,
   dvcLiveOnly: boolean
 ): ExperimentsAccumulator => {
   const acc: ExperimentsAccumulator = {
+    cliError: undefined,
     commits: [],
     experimentsByCommit: new Map(),
     hasCheckpoints: hasCheckpoints(expShow),
@@ -363,6 +374,8 @@ export const collectExperiments = (
 
   setWorkspaceAsRunning(acc, dvcLiveOnly)
 
+  collectCliError(acc, expShow)
+
   return acc
 }
 

extension/src/experiments/model/index.test.ts

@@ -522,4 +522,32 @@ describe('ExperimentsModel', () => {
     expect(getSelectedRevisions(model)).toStrictEqual([])
     expect(model.hasRunningExperiment()).toBe(false)
   })
+
+  it('should capture a Cli error when exp show fails', () => {
+    const model = new ExperimentsModel('', buildMockMemento())
+
+    const errorMsg = 'a very unexpected error - (╯°□°）╯︵ ┻━┻'
+
+    const data = [
+      {
+        rev: EXPERIMENT_WORKSPACE_ID,
+        error: { msg: errorMsg, type: 'caught error' }
+      }
+    ]
+    model.transformAndSet(data, gitLogFixture, false, [], {
+      main: 6
+    })
+
+    expect(model.getCliError()).toStrictEqual(errorMsg)
+
+    model.transformAndSet(
+      outputFixture,
+      gitLogFixture,
+      false,
+      rowOrderFixture,
+      { main: 6 }
+    )
+
+    expect(model.getCliError()).toBe(undefined)
+  })
 })

extension/src/experiments/model/index.ts

@@ -53,6 +53,7 @@ export enum ExperimentType {
 
 export class ExperimentsModel extends ModelWithPersistence {
   private workspace = {} as Experiment
+  private cliError: undefined | string
   private commits: Experiment[] = []
   private experimentsByCommit: Map<string, Experiment[]> = new Map()
   private rowOrder: { branch: string; sha: string }[] = []
@@ -122,11 +123,12 @@ export class ExperimentsModel extends ModelWithPersistence {
     availableNbCommits: { [branch: string]: number }
   ) {
     const {
-      workspace,
+      cliError,
       commits,
       experimentsByCommit,
+      hasCheckpoints,
       runningExperiments,
-      hasCheckpoints
+      workspace
     } = collectExperiments(expShow, gitLog, dvcLiveOnly)
 
     const { hasMoreCommits, isShowingMoreCommits } =
@@ -141,6 +143,7 @@ export class ExperimentsModel extends ModelWithPersistence {
 
     this.rowOrder = rowOrder
     this.workspace = workspace
+    this.cliError = cliError
     this.commits = commits
     this.experimentsByCommit = experimentsByCommit
     this.checkpoints = hasCheckpoints
@@ -192,6 +195,10 @@ export class ExperimentsModel extends ModelWithPersistence {
     return this.checkpoints
   }
 
+  public getCliError() {
+    return this.cliError
+  }
+
   public canSelect() {
     return canSelect(this.coloredStatus)
   }
@@ -323,11 +330,18 @@ export class ExperimentsModel extends ModelWithPersistence {
   }
 
   public getErrors() {
-    return new Set(
-      this.getCombinedList()
-        .filter(({ error }) => error)
-        .map(({ label }) => label)
-    )
+    const errors = new Set<string>()
+    for (const { error, label } of this.getCombinedList()) {
+      if (!error) {
+        continue
+      }
+      errors.add(label)
+    }
+    if (this.cliError) {
+      errors.add(this.cliError)
+    }
+
+    return errors
   }
 
   public getExperimentParams(id: string) {

extension/src/experiments/model/tree.test.ts

@@ -34,6 +34,7 @@ const mockedGetMarkdownString = jest.mocked(getMarkdownString)
 const {
   mockedExperiments,
   mockedGetCommitExperiments,
+  mockedGetCliError,
   mockedGetDvcRoots,
   mockedGetFirstThreeColumnOrder,
   mockedGetWorkspaceAndCommits
@@ -450,6 +451,37 @@ describe('ExperimentsTree', () => {
         }
       ])
     })
+
+    it('should return an error item if the repository has a CLI error', async () => {
+      const errorMsg = 'the CLI is broken :('
+      mockedGetCliError.mockReturnValueOnce(errorMsg)
+      const experiments = [
+        {
+          displayColor: undefined,
+          error: errorMsg,
+          hasChildren: false,
+          id: EXPERIMENT_WORKSPACE_ID,
+          label: EXPERIMENT_WORKSPACE_ID,
+          selected: false,
+          type: ExperimentType.WORKSPACE
+        }
+      ]
+
+      mockedGetWorkspaceAndCommits
+        .mockReturnValueOnce(experiments)
+        .mockReturnValueOnce(experiments)
+      mockedGetDvcRoots.mockReturnValueOnce(['repo'])
+      mockedGetFirstThreeColumnOrder.mockReturnValue([])
+
+      const experimentsTree = new ExperimentsTree(
+        mockedExperiments,
+        mockedResourceLocator
+      )
+
+      const children = await experimentsTree.getChildren()
+
+      expect(children).toStrictEqual([{ error: errorMsg }])
+    })
   })
 
   describe('getTreeItem', () => {

extension/src/experiments/model/tree.ts

@@ -19,9 +19,12 @@ import { definedAndNonEmpty, sortCollectedArray } from '../../util/array'
 import {
   createTreeView,
   DecoratableTreeItemScheme,
+  ErrorItem,
+  getCliErrorTreeItem,
   getDecoratableTreeItem,
   getErrorTooltip,
   getRootItem,
+  isErrorItem,
   isRoot
 } from '../../tree'
 import { IconName, Resource, ResourceLocator } from '../../resourceLocator'
@@ -44,14 +47,14 @@ type ExperimentAugmented = Experiment & {
 
 export class ExperimentsTree
   extends Disposable
-  implements TreeDataProvider<string | ExperimentItem>
+  implements TreeDataProvider<string | ExperimentItem | ErrorItem>
 {
   public readonly onDidChangeTreeData: Event<string | void>
 
   private readonly experiments: WorkspaceExperiments
   private readonly resourceLocator: ResourceLocator
 
-  private readonly view: TreeView<string | ExperimentItem>
+  private readonly view: TreeView<string | ExperimentItem | ErrorItem>
   private viewed = false
 
   constructor(
@@ -63,7 +66,11 @@ export class ExperimentsTree
     this.onDidChangeTreeData = experiments.experimentsChanged.event
 
     this.view = this.dispose.track(
-      createTreeView<ExperimentItem>('dvc.views.experimentsTree', this, true)
+      createTreeView<ExperimentItem | ErrorItem>(
+        'dvc.views.experimentsTree',
+        this,
+        true
+      )
     )
 
     this.experiments = experiments
@@ -79,6 +86,15 @@ export class ExperimentsTree
       return getRootItem(element)
     }
 
+    if (isErrorItem(element)) {
+      const { error } = element
+      return getCliErrorTreeItem(
+        error,
+        error,
+        DecoratableTreeItemScheme.EXPERIMENTS
+      )
+    }
+
     const {
       label,
       collapsibleState,
@@ -109,7 +125,7 @@ export class ExperimentsTree
 
   public getChildren(
     element?: string | ExperimentItem
-  ): Promise<string[] | ExperimentItem[]> {
+  ): Promise<string[] | ExperimentItem[] | ErrorItem[]> {
     if (!element) {
       return this.getRootElements()
     }
@@ -226,9 +242,17 @@ export class ExperimentsTree
     }
   }
 
-  private getWorkspaceAndCommits(dvcRoot: string): ExperimentItem[] {
-    return this.experiments
-      .getRepository(dvcRoot)
+  private getWorkspaceAndCommits(
+    dvcRoot: string
+  ): ExperimentItem[] | ErrorItem[] {
+    const repository = this.experiments.getRepository(dvcRoot)
+
+    const cliError = repository.getCliError()
+    if (cliError) {
+      return [{ error: cliError }]
+    }
+
+    return repository
       .getWorkspaceAndCommits()
       .map(experiment => this.formatExperiment(experiment, dvcRoot))
   }

extension/src/experiments/webview/contract.ts

@@ -93,6 +93,7 @@ export interface Column extends ColumnAggregateData {
 
 export type TableData = {
   changes: string[]
+  cliError: string | undefined
   columnOrder: string[]
   columns: Column[]
   columnWidths: Record<string, number>

extension/src/experiments/webview/messages.ts

@@ -205,6 +205,9 @@ export class WebviewMessages {
       case MessageFromWebviewType.SELECT_BRANCHES:
         return this.addAndRemoveBranches()
 
+      case MessageFromWebviewType.REFRESH_EXP_DATA:
+        return this.refreshData()
+
       default:
         Logger.error(`Unexpected message: ${JSON.stringify(message)}`)
     }
@@ -242,9 +245,20 @@ export class WebviewMessages {
     await this.update()
   }
 
+  private refreshData() {
+    sendTelemetryEvent(
+      EventName.VIEWS_EXPERIMENTS_TABLE_REFRESH,
+      undefined,
+      undefined
+    )
+
+    return this.update()
+  }
+
   private getWebviewData() {
     return {
       changes: this.columns.getChanges(),
+      cliError: this.experiments.getCliError(),
       columnOrder: this.columns.getColumnOrder(),
       columnWidths: this.columns.getColumnWidths(),
       columns: this.columns.getSelected(),

extension/src/experiments/workspace.ts

@@ -386,6 +386,19 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
     return allLoading
   }
 
+  public getCliError() {
+    const repositories = Object.values(this.repositories)
+    const errors = []
+    for (const repository of repositories) {
+      const cliError = repository.getCliError()
+      if (!cliError) {
+        continue
+      }
+      errors.push(cliError)
+    }
+    return errors.length > 0 ? errors.join('\n') : undefined
+  }
+
   public hasRunningExperiment() {
     return Object.values(this.repositories).some(experiments =>
       experiments.hasRunningExperiment()

extension/src/setup/index.ts

@@ -74,6 +74,7 @@ export class Setup
 
   private readonly webviewMessages: WebviewMessages
   private readonly getHasData: () => boolean | undefined
+  private readonly getExpShowError: () => string | undefined
   private readonly collectWorkspaceScale: () => Promise<WorkspaceScale>
 
   private readonly workspaceChanged: EventEmitter<void> = this.dispose.track(
@@ -129,6 +130,7 @@ export class Setup
     }
 
     this.getHasData = () => experiments.getHasData()
+    this.getExpShowError = () => experiments.getCliError()
     const onDidChangeHasData = experiments.columnsChanged.event
     this.dispose.track(
       onDidChangeHasData(() =>
@@ -232,7 +234,7 @@ export class Setup
   public shouldBeShown(): { dvc: boolean; experiments: boolean } {
     return {
       dvc: !!this.getCliCompatible() && this.hasRoots(),
-      experiments: !!this.getHasData()
+      experiments: !!(this.getExpShowError() || this.getHasData())
     }
   }