Do not process CLI errors thrown by plots diff (#2852)

Author: mattseddon

extension/src/cli/dvc/contract.ts

@@ -93,3 +93,5 @@ export interface ExperimentsOutput {
 export interface PlotsOutput {
   [path: string]: Plot[]
 }
+
+export type PlotsOutputOrError = PlotsOutput | DvcError

extension/src/cli/dvc/reader.ts

@@ -13,7 +13,8 @@ import {
   DataStatusOutput,
   DvcError,
   ExperimentsOutput,
-  PlotsOutput
+  PlotsOutput,
+  PlotsOutputOrError
 } from './contract'
 import { getOptions } from './options'
 import { typeCheckCommands } from '..'
@@ -29,7 +30,8 @@ export const isDvcError = <
   T extends ExperimentsOutput | DataStatusOutput | PlotsOutput
 >(
   dataOrError: T | DvcError
-): dataOrError is DvcError => !!(dataOrError as DvcError).error
+): dataOrError is DvcError =>
+  !!(Object.keys(dataOrError).length === 1 && (dataOrError as DvcError).error)
 
 export const autoRegisteredCommands = {
   DATA_STATUS: 'dataStatus',
@@ -80,7 +82,7 @@ export class DvcReader extends DvcCli {
   public plotsDiff(
     cwd: string,
     ...revisions: string[]
-  ): Promise<PlotsOutput | DvcError> {
+  ): Promise<PlotsOutputOrError> {
     return this.readProcessJson<PlotsOutput>(
       cwd,
       Command.PLOTS,
@@ -127,7 +129,7 @@ export class DvcReader extends DvcCli {
     } catch (error: unknown) {
       const msg =
         (error as MaybeConsoleError).stderr || (error as Error).message
-      Logger.error(`${args} failed with ${msg} retrying...`)
+      Logger.error(`${args} failed with ${msg}`)
       return { error: { msg, type: 'Caught error' } }
     }
   }

extension/src/data/index.ts

@@ -7,12 +7,12 @@ import {
 } from '../fileSystem/watcher'
 import { ProcessManager } from '../processManager'
 import { InternalCommands } from '../commands/internal'
-import { ExperimentsOutput, PlotsOutput } from '../cli/dvc/contract'
+import { ExperimentsOutput, PlotsOutputOrError } from '../cli/dvc/contract'
 import { definedAndNonEmpty, sameContents, uniqueValues } from '../util/array'
 import { DeferredDisposable } from '../class/deferred'
 
 export abstract class BaseData<
-  T extends { data: PlotsOutput; revs: string[] } | ExperimentsOutput
+  T extends { data: PlotsOutputOrError; revs: string[] } | ExperimentsOutput
 > extends DeferredDisposable {
   public readonly onDidUpdate: Event<T>
 

extension/src/plots/data/index.ts

@@ -1,5 +1,6 @@
 import { EventEmitter } from 'vscode'
-import { PlotsOutput } from '../../cli/dvc/contract'
+import { PlotsOutputOrError } from '../../cli/dvc/contract'
+import { isDvcError } from '../../cli/dvc/reader'
 import { AvailableCommands, InternalCommands } from '../../commands/internal'
 import { BaseData } from '../../data'
 import {
@@ -9,7 +10,10 @@ import {
 } from '../../util/array'
 import { PlotsModel } from '../model'
 
-export class PlotsData extends BaseData<{ data: PlotsOutput; revs: string[] }> {
+export class PlotsData extends BaseData<{
+  data: PlotsOutputOrError
+  revs: string[]
+}> {
   private readonly model: PlotsModel
 
   constructor(
@@ -49,7 +53,7 @@ export class PlotsData extends BaseData<{ data: PlotsOutput; revs: string[] }> {
     }
 
     const args = this.getArgs(revs)
-    const data = await this.internalCommands.executeCommand<PlotsOutput>(
+    const data = await this.internalCommands.executeCommand<PlotsOutputOrError>(
       AvailableCommands.PLOTS_DIFF,
       this.dvcRoot,
       ...args
@@ -66,8 +70,11 @@ export class PlotsData extends BaseData<{ data: PlotsOutput; revs: string[] }> {
     return this.processManager.run('update')
   }
 
-  public collectFiles({ data }: { data: PlotsOutput }) {
-    return Object.keys(data)
+  public collectFiles({ data }: { data: PlotsOutputOrError }) {
+    if (isDvcError(data)) {
+      return this.collectedFiles
+    }
+    return [...Object.keys(data), ...this.collectedFiles]
   }
 
   private getArgs(revs: string[]) {

extension/src/plots/model/index.ts

@@ -25,7 +25,7 @@ import {
   SectionCollapsed,
   PlotSizeNumber
 } from '../webview/contract'
-import { ExperimentsOutput, PlotsOutput } from '../../cli/dvc/contract'
+import { ExperimentsOutput, PlotsOutputOrError } from '../../cli/dvc/contract'
 import { Experiments } from '../../experiments'
 import { getColorScale, truncateVerticalTitle } from '../vega/util'
 import { definedAndNonEmpty, reorderObjectList } from '../../util/array'
@@ -39,6 +39,7 @@ import {
   MultiSourceEncoding,
   MultiSourceVariations
 } from '../multiSource/collect'
+import { isDvcError } from '../../cli/dvc/reader'
 
 export class PlotsModel extends ModelWithPersistence {
   private readonly experiments: Experiments
@@ -104,7 +105,11 @@ export class PlotsModel extends ModelWithPersistence {
     return this.removeStaleData()
   }
 
-  public async transformAndSetPlots(data: PlotsOutput, revs: string[]) {
+  public async transformAndSetPlots(data: PlotsOutputOrError, revs: string[]) {
+    if (isDvcError(data)) {
+      return
+    }
+
     const cliIdToLabel = this.getCLIIdToLabel()
 
     this.fetchedRevs = new Set([

extension/src/plots/paths/model.test.ts

@@ -257,4 +257,16 @@ describe('PathsModel', () => {
     const noChildren = model.getChildren(logsLoss)
     expect(noChildren).toStrictEqual([])
   })
+
+  it('should not provide error as a path when the CLI throws an error', () => {
+    const model = new PathsModel(mockDvcRoot, buildMockMemento())
+    model.transformAndSet({
+      error: {
+        msg: 'UNEXPECTED ERROR: a strange thing happened',
+        type: 'Caught Error'
+      }
+    })
+
+    expect(model.getTerminalNodes()).toStrictEqual([])
+  })
 })

extension/src/plots/paths/model.ts

@@ -6,11 +6,12 @@ import {
   PlotPath,
   TemplateOrder
 } from './collect'
-import { PlotsOutput } from '../../cli/dvc/contract'
 import { PathSelectionModel } from '../../path/selection/model'
 import { PersistenceKey } from '../../persistence/constants'
 import { performSimpleOrderedUpdate } from '../../util/array'
 import { MultiSourceEncoding } from '../multiSource/collect'
+import { isDvcError } from '../../cli/dvc/reader'
+import { PlotsOutputOrError } from '../../cli/dvc/contract'
 
 export class PathsModel extends PathSelectionModel<PlotPath> {
   private templateOrder: TemplateOrder
@@ -26,7 +27,11 @@ export class PathsModel extends PathSelectionModel<PlotPath> {
     )
   }
 
-  public transformAndSet(data: PlotsOutput) {
+  public transformAndSet(data: PlotsOutputOrError) {
+    if (isDvcError(data)) {
+      return
+    }
+
     const paths = collectPaths(this.data, data)
 
     this.setNewStatuses(paths)