From 4be1b8d0cdc5d3e04b29908688a0576818a44679 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 29 Nov 2022 13:27:07 +1100 Subject: [PATCH] Do not process CLI errors thrown by plots diff --- extension/src/cli/dvc/reader.ts | 5 +++-- extension/src/plots/model/index.ts | 16 ++++++++++++++-- extension/src/plots/paths/model.test.ts | 12 ++++++++++++ extension/src/plots/paths/model.ts | 9 +++++++-- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/extension/src/cli/dvc/reader.ts b/extension/src/cli/dvc/reader.ts index 4ef291076c..ab2a332f48 100644 --- a/extension/src/cli/dvc/reader.ts +++ b/extension/src/cli/dvc/reader.ts @@ -29,7 +29,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', @@ -127,7 +128,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' } } } } diff --git a/extension/src/plots/model/index.ts b/extension/src/plots/model/index.ts index d0f44cc154..2d206ccf0e 100644 --- a/extension/src/plots/model/index.ts +++ b/extension/src/plots/model/index.ts @@ -25,7 +25,11 @@ import { SectionCollapsed, PlotSizeNumber } from '../webview/contract' -import { ExperimentsOutput, PlotsOutput } from '../../cli/dvc/contract' +import { + DvcError, + ExperimentsOutput, + PlotsOutput +} from '../../cli/dvc/contract' import { Experiments } from '../../experiments' import { getColorScale, truncateVerticalTitle } from '../vega/util' import { definedAndNonEmpty, reorderObjectList } from '../../util/array' @@ -39,6 +43,7 @@ import { MultiSourceEncoding, MultiSourceVariations } from '../multiSource/collect' +import { isDvcError } from '../../cli/dvc/reader' export class PlotsModel extends ModelWithPersistence { private readonly experiments: Experiments @@ -104,7 +109,14 @@ export class PlotsModel extends ModelWithPersistence { return this.removeStaleData() } - public async transformAndSetPlots(data: PlotsOutput, revs: string[]) { + public async transformAndSetPlots( + data: PlotsOutput | DvcError, + revs: string[] + ) { + if (isDvcError(data)) { + return + } + const cliIdToLabel = this.getCLIIdToLabel() this.fetchedRevs = new Set([ diff --git a/extension/src/plots/paths/model.test.ts b/extension/src/plots/paths/model.test.ts index bef3996e51..8d11422a23 100644 --- a/extension/src/plots/paths/model.test.ts +++ b/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([]) + }) }) diff --git a/extension/src/plots/paths/model.ts b/extension/src/plots/paths/model.ts index 43d015fc0d..75d5b61f2a 100644 --- a/extension/src/plots/paths/model.ts +++ b/extension/src/plots/paths/model.ts @@ -6,11 +6,12 @@ import { PlotPath, TemplateOrder } from './collect' -import { PlotsOutput } from '../../cli/dvc/contract' +import { DvcError, 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' export class PathsModel extends PathSelectionModel { private templateOrder: TemplateOrder @@ -26,7 +27,11 @@ export class PathsModel extends PathSelectionModel { ) } - public transformAndSet(data: PlotsOutput) { + public transformAndSet(data: PlotsOutput | DvcError) { + if (isDvcError(data)) { + return + } + const paths = collectPaths(this.data, data) this.setNewStatuses(paths)