Display error thrown by Plots Diff in Plots tree (#3569)

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

@@ -439,7 +439,7 @@ describe('ExperimentsColumnsTree', () => {
     it('should return the correct tree item for a repository root', () => {
       let mockedItem = {}
       mockedTreeItem.mockImplementationOnce(function (uri, collapsibleState) {
-        expect(collapsibleState).toStrictEqual(1)
+        expect(collapsibleState).toStrictEqual(2)
         mockedItem = { collapsibleState, uri }
         return mockedItem
       })
@@ -498,6 +498,7 @@ describe('ExperimentsColumnsTree', () => {
       },
       description: '3/4',
       iconPath: mockedSelectedCheckbox,
+      tooltip: undefined,
       uri: filename
     })
   })
@@ -538,6 +539,7 @@ describe('ExperimentsColumnsTree', () => {
         title: 'toggle'
       },
       iconPath: mockedEmptyCheckbox,
+      tooltip: undefined,
       uri: filename
     })
   })

extension/src/experiments/columns/tree.ts

@@ -8,6 +8,7 @@ import { ResourceLocator } from '../../resourceLocator'
 import { RegisteredCommands } from '../../commands/external'
 import { EventName } from '../../telemetry/constants'
 import { InternalCommands } from '../../commands/internal'
+import { getRootItem, isRoot } from '../../tree'
 
 export class ExperimentsColumnsTree extends BasePathSelectionTree<WorkspaceExperiments> {
   constructor(
@@ -31,11 +32,16 @@ export class ExperimentsColumnsTree extends BasePathSelectionTree<WorkspaceExper
     )
   }
 
-  protected getBaseTreeItem({
-    label,
-    collapsibleState
-  }: PathSelectionItem & { label: string }) {
-    return new TreeItem(label, collapsibleState)
+  public getTreeItem(element: string | PathSelectionItem): TreeItem {
+    if (isRoot(element)) {
+      return getRootItem(element)
+    }
+
+    const { label, collapsibleState } = element
+
+    const treeItem = new TreeItem(label as string, collapsibleState)
+
+    return this.addTreeItemDetails(element, treeItem)
   }
 
   protected getRepositoryChildren(dvcRoot: string, path: string) {

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

@@ -18,7 +18,7 @@ import {
   joinTruthyItems,
   sortCollectedArray
 } from '../../../util/array'
-import { createTreeView, getRootItem } from '../../../tree'
+import { createTreeView, getRootItem, isRoot } from '../../../tree'
 import { Disposable } from '../../../class/dispose'
 
 export type FilterItem = {
@@ -66,7 +66,7 @@ export class ExperimentsFilterByTree
   }
 
   public getTreeItem(element: string | FilterItem): TreeItem {
-    if (this.isRoot(element)) {
+    if (isRoot(element)) {
       return getRootItem(element)
     }
 
@@ -144,10 +144,6 @@ export class ExperimentsFilterByTree
       .removeFilter(filter.id)
   }
 
-  private isRoot(element: string | FilterItem): element is string {
-    return typeof element === 'string'
-  }
-
   private getDvcRoots() {
     return this.experiments.getDvcRoots()
   }

extension/src/experiments/model/tree.ts

@@ -22,7 +22,8 @@ import {
   DecoratableTreeItemScheme,
   getDecoratableTreeItem,
   getErrorTooltip,
-  getRootItem
+  getRootItem,
+  isRoot
 } from '../../tree'
 import { IconName, Resource, ResourceLocator } from '../../resourceLocator'
 import {
@@ -82,7 +83,7 @@ export class ExperimentsTree
   }
 
   public getTreeItem(element: string | ExperimentItem): TreeItem {
-    if (this.isRoot(element)) {
+    if (isRoot(element)) {
       return getRootItem(element)
     }
 
@@ -121,7 +122,7 @@ export class ExperimentsTree
       return this.getRootElements()
     }
 
-    if (this.isRoot(element)) {
+    if (isRoot(element)) {
       return Promise.resolve(this.getWorkspaceAndCommits(element))
     }
 
@@ -232,7 +233,7 @@ export class ExperimentsTree
   }
 
   private setExpanded(element: string | ExperimentItem, expanded: boolean) {
-    if (!this.isRoot(element) && element.description) {
+    if (!isRoot(element) && element.description) {
       this.setExperimentExpanded(element.description, expanded)
     }
   }
@@ -351,10 +352,6 @@ export class ExperimentsTree
     )
   }
 
-  private isRoot(element: string | ExperimentItem): element is string {
-    return typeof element === 'string'
-  }
-
   private getSelectedExperimentItems() {
     return [...this.view.selection]
   }

extension/src/path/selection/model.ts

@@ -46,7 +46,7 @@ export abstract class PathSelectionModel<
 
   public getTerminalNodeStatuses(parentPath?: string): Status[] {
     return (this.getChildren(parentPath) || []).flatMap(element => {
-      const terminalStatuses = element.hasChildren
+      const terminalStatuses = (element as T).hasChildren
         ? this.getTerminalNodeStatuses(element.path)
         : [this.status[element.path]]
       return [...terminalStatuses]
@@ -144,7 +144,9 @@ export abstract class PathSelectionModel<
 
   abstract getChildren(
     ...args: unknown[]
-  ): (T & { descendantStatuses: Status[]; status: Status })[]
+  ):
+    | (T & { descendantStatuses: Status[]; status: Status })[]
+    | { error: string; path: string }[]
 
   abstract getTerminalNodes(): (T & { selected: boolean })[]
 }

extension/src/path/selection/tree.ts

@@ -12,34 +12,38 @@ import { WorkspaceExperiments } from '../../experiments/workspace'
 import { WorkspacePlots } from '../../plots/workspace'
 import { Resource, ResourceLocator } from '../../resourceLocator'
 import { RegisteredCommands } from '../../commands/external'
-import { createTreeView } from '../../tree'
+import { createTreeView, isRoot } from '../../tree'
 import { definedAndNonEmpty, sortCollectedArray } from '../../util/array'
 import { sendViewOpenedTelemetryEvent } from '../../telemetry'
 import { ViewOpenedEventName } from '../../telemetry/constants'
 import { Disposable } from '../../class/dispose'
 
+export type ErrorItem = { error: string; path: string }
+
 export type PathSelectionItem = {
+  collapsibleState: TreeItemCollapsibleState
   description: string | undefined
   dvcRoot: string
-  collapsibleState: TreeItemCollapsibleState
+  iconPath: Resource | Uri
   label: string | undefined
   path: string
-  iconPath: Resource | Uri
   tooltip: MarkdownString | undefined
 }
 
+type Item = ErrorItem | PathSelectionItem
+
 export abstract class BasePathSelectionTree<
     T extends WorkspaceExperiments | WorkspacePlots
   >
   extends Disposable
-  implements TreeDataProvider<string | PathSelectionItem>
+  implements TreeDataProvider<string | Item>
 {
   public readonly onDidChangeTreeData: Event<PathSelectionItem | void>
 
   protected readonly workspace: T
   protected readonly resourceLocator: ResourceLocator
 
-  private readonly view: TreeView<string | PathSelectionItem>
+  private readonly view: TreeView<string | Item>
 
   private viewed = false
   private readonly openEventName: ViewOpenedEventName
@@ -61,9 +65,7 @@ export abstract class BasePathSelectionTree<
 
     this.onDidChangeTreeData = changeEvent
 
-    this.view = this.dispose.track(
-      createTreeView<PathSelectionItem>(name, this)
-    )
+    this.view = this.dispose.track(createTreeView<Item>(name, this))
 
     this.toggleCommand = toggleCommand
 
@@ -72,44 +74,17 @@ export abstract class BasePathSelectionTree<
     this.updateDescriptionOnChange()
   }
 
-  public getTreeItem(element: string | PathSelectionItem): TreeItem {
-    if (this.isRoot(element)) {
-      const resourceUri = Uri.file(element)
-      return new TreeItem(resourceUri, TreeItemCollapsibleState.Collapsed)
-    }
-
-    const { dvcRoot, path, description, iconPath, tooltip } = element
-
-    const treeItem = this.getBaseTreeItem(element)
-
-    treeItem.command = {
-      arguments: [{ dvcRoot, path }],
-      command: this.toggleCommand,
-      title: 'toggle'
-    }
-
-    treeItem.iconPath = iconPath
-    if (description) {
-      treeItem.description = description
-    }
-    if (tooltip) {
-      treeItem.tooltip = tooltip
-    }
-
-    return treeItem
-  }
-
   public getChildren(
     element?: string | PathSelectionItem
-  ): Promise<PathSelectionItem[] | string[]> {
+  ): Promise<Item[] | string[]> {
     if (element) {
       return Promise.resolve(this.getChildElements(element))
     }
 
     return this.getRootElements()
   }
 
-  protected getIconPath(status?: Status) {
+  protected getIconPath(status: Status) {
     if (status === Status.SELECTED) {
       return this.resourceLocator.checkedCheckbox
     }
@@ -155,7 +130,7 @@ export abstract class BasePathSelectionTree<
       ? TreeItemCollapsibleState.Collapsed
       : TreeItemCollapsibleState.None
 
-    return {
+    const pathSelectionItem: PathSelectionItem = {
       collapsibleState,
       description,
       dvcRoot,
@@ -164,6 +139,27 @@ export abstract class BasePathSelectionTree<
       path,
       tooltip
     }
+
+    return pathSelectionItem
+  }
+
+  protected addTreeItemDetails(element: PathSelectionItem, treeItem: TreeItem) {
+    const { dvcRoot, path, description, iconPath, tooltip } = element
+
+    treeItem.command = {
+      arguments: [{ dvcRoot, path }],
+      command: this.toggleCommand,
+      title: 'toggle'
+    }
+
+    treeItem.iconPath = iconPath
+    if (description) {
+      treeItem.description = description
+    }
+
+    treeItem.tooltip = tooltip
+
+    return treeItem
   }
 
   private updateDescriptionOnChange() {
@@ -196,14 +192,12 @@ export abstract class BasePathSelectionTree<
     return sortCollectedArray(dvcRoots, (a, b) => a.localeCompare(b))
   }
 
-  private getChildElements(
-    element: string | PathSelectionItem
-  ): PathSelectionItem[] {
+  private getChildElements(element: string | PathSelectionItem): Item[] {
     if (!element) {
       return []
     }
 
-    if (this.isRoot(element)) {
+    if (isRoot(element)) {
       return this.getRepositoryChildren(element, undefined)
     }
 
@@ -212,16 +206,12 @@ export abstract class BasePathSelectionTree<
     return this.getRepositoryChildren(dvcRoot, path)
   }
 
-  private isRoot(element: string | PathSelectionItem): element is string {
-    return typeof element === 'string'
-  }
-
-  protected abstract getBaseTreeItem(element: PathSelectionItem): TreeItem
+  abstract getTreeItem(element: string | Item): TreeItem
 
   protected abstract getRepositoryChildren(
     dvcRoot: string,
     path: string | undefined
-  ): PathSelectionItem[]
+  ): Item[]
 
   protected abstract getRepositoryStatuses(dvcRoot: string): Status[]
 }

extension/src/plots/errors/model.ts

@@ -5,29 +5,32 @@ import {
   collectPathErrorsTable
 } from './collect'
 import { Disposable } from '../../class/dispose'
-import { PlotError, PlotsOutputOrError } from '../../cli/dvc/contract'
+import { DvcError, PlotError, PlotsOutputOrError } from '../../cli/dvc/contract'
 import { isDvcError } from '../../cli/dvc/reader'
+import { getCliErrorLabel } from '../../tree'
 
 export class ErrorsModel extends Disposable {
   private readonly dvcRoot: string
 
   private errors: PlotError[] = []
+  private cliError: { error: string; path: string } | undefined
 
   constructor(dvcRoot: string) {
     super()
     this.dvcRoot = dvcRoot
   }
 
   public transformAndSet(
-    data: PlotsOutputOrError,
+    output: PlotsOutputOrError,
     revs: string[],
     cliIdToLabel: { [id: string]: string }
   ) {
-    if (isDvcError(data)) {
-      return
+    if (isDvcError(output)) {
+      return this.handleCliError(output)
     }
 
-    this.errors = collectErrors(data, revs, this.errors, cliIdToLabel)
+    this.errors = collectErrors(output, revs, this.errors, cliIdToLabel)
+    this.cliError = undefined
   }
 
   public getImageErrors(path: string, revision: string) {
@@ -39,6 +42,10 @@ export class ErrorsModel extends Disposable {
   }
 
   public getErrorPaths(selectedRevisions: string[]) {
+    if (this.cliError) {
+      return new Set([this.cliError.path])
+    }
+
     const acc = new Set<string>()
     for (const { name, rev } of this.errors) {
       if (selectedRevisions.includes(rev)) {
@@ -47,4 +54,20 @@ export class ErrorsModel extends Disposable {
     }
     return acc
   }
+
+  public hasCliError() {
+    return !!this.getCliError()
+  }
+
+  public getCliError() {
+    return this.cliError
+  }
+
+  private handleCliError({ error: { msg } }: DvcError) {
+    this.errors = []
+    this.cliError = {
+      error: msg,
+      path: join(this.dvcRoot, getCliErrorLabel(msg))
+    }
+  }
 }