any retry and return a default when error is unexpected

iterative · Aug 17, 2022 · 873b149 · 873b149
1 parent 36571f5
commit 873b149
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 51 deletions.
diff --git a/extension/src/cli/constants.ts b/extension/src/cli/constants.ts
@@ -2,7 +2,7 @@ export const MIN_CLI_VERSION = '2.11.0'
 export const LATEST_TESTED_CLI_VERSION = '2.13.0'
 export const MAX_CLI_VERSION = '3'
 
-export const EMPTY_REPO_ERROR = 'unexpected error - Empty git repo'
+export const UNEXPECTED_ERROR_CODE = 255
 
 export enum Command {
   ADD = 'add',

diff --git a/extension/src/cli/reader.test.ts b/extension/src/cli/reader.test.ts
@@ -2,7 +2,8 @@ import { join } from 'path'
 import { EventEmitter } from 'vscode'
 import { Disposable, Disposer } from '@hediet/std/disposable'
 import { CliResult, CliStarted } from '.'
-import { EMPTY_REPO_ERROR } from './constants'
+import { UNEXPECTED_ERROR_CODE } from './constants'
+import { MaybeConsoleError } from './error'
 import { CliReader, ExperimentsOutput } from './reader'
 import { createProcess } from '../processExecution'
 import { getFailingMockedProcess, getMockedProcess } from '../test/util/jest'
@@ -80,8 +81,10 @@ describe('CliReader', () => {
 
     it('should return the default output if the cli returns an empty repo error', async () => {
       const cwd = __dirname
+      const error = new Error('unexpected error - something something')
+      ;(error as MaybeConsoleError).exitCode = UNEXPECTED_ERROR_CODE
       mockedCreateProcess.mockImplementationOnce(() => {
-        throw new Error(EMPTY_REPO_ERROR)
+        throw error
       })
 
       const cliOutput = await cliReader.expShow(cwd)

diff --git a/extension/src/cli/reader.ts b/extension/src/cli/reader.ts
@@ -3,7 +3,6 @@ import { Cli, typeCheckCommands } from '.'
 import {
   Args,
   Command,
-  EMPTY_REPO_ERROR,
   ExperimentFlag,
   Flag,
   ListFlag,
@@ -137,13 +136,14 @@ export class CliReader extends Cli {
     cwd: string,
     ...flags: ExperimentFlag[]
   ): Promise<ExperimentsOutput> {
-    return this.readProcessJsonWithKnownErrors<ExperimentsOutput>(
+    return this.readProcess<ExperimentsOutput>(
       cwd,
+      JSON.parse,
+      JSON.stringify(defaultExperimentsOutput),
       Command.EXPERIMENT,
-      defaultExperimentsOutput,
-      [EMPTY_REPO_ERROR],
       SubCommand.SHOW,
-      ...flags
+      ...flags,
+      Flag.SHOW_JSON
     )
   }
 
@@ -162,11 +162,9 @@ export class CliReader extends Cli {
   }
 
   public plotsDiff(cwd: string, ...revisions: string[]): Promise<PlotsOutput> {
-    return this.readProcessJsonWithKnownErrors<PlotsOutput>(
+    return this.readProcessJson<PlotsOutput>(
       cwd,
       Command.PLOTS,
-      {},
-      [EMPTY_REPO_ERROR],
       Command.DIFF,
       ...revisions,
       Flag.OUTPUT_PATH,
@@ -193,15 +191,11 @@ export class CliReader extends Cli {
     cwd: string,
     formatter: typeof trimAndSplit | typeof JSON.parse | typeof trim,
     defaultValue: string,
-    nonRetryErrors: string[],
     ...args: Args
   ): Promise<T> {
     const output =
-      (await retry(
-        () => this.executeProcess(cwd, ...args),
-        args.join(' '),
-        nonRetryErrors
-      )) || defaultValue
+      (await retry(() => this.executeProcess(cwd, ...args), args.join(' '))) ||
+      defaultValue
     if (!formatter) {
       return output as unknown as T
     }
@@ -213,25 +207,6 @@ export class CliReader extends Cli {
       cwd,
       JSON.parse,
       '{}',
-      [],
-      command,
-      ...args,
-      Flag.SHOW_JSON
-    )
-  }
-
-  private readProcessJsonWithKnownErrors<T>(
-    cwd: string,
-    command: Command,
-    defaultValue: T,
-    nonRetryErrors: string[],
-    ...args: Args
-  ) {
-    return this.readProcess<T>(
-      cwd,
-      JSON.parse,
-      JSON.stringify(defaultValue),
-      nonRetryErrors,
       command,
       ...args,
       Flag.SHOW_JSON

diff --git a/extension/src/cli/retry.test.ts b/extension/src/cli/retry.test.ts
@@ -17,7 +17,7 @@ describe('retry', () => {
 
     const promiseRefresher = jest.fn().mockImplementation(() => promise())
 
-    const output = await retry(promiseRefresher, 'Definitely did not', [])
+    const output = await retry(promiseRefresher, 'Definitely did not')
 
     expect(output).toStrictEqual(returnValue)
 
@@ -45,7 +45,7 @@ describe('retry', () => {
       .fn()
       .mockImplementation(() => unreliablePromise())
 
-    await retry(promiseRefresher, 'Data update', [])
+    await retry(promiseRefresher, 'Data update')
 
     expect(promiseRefresher).toBeCalledTimes(4)
     expect(mockedDelay).toBeCalledTimes(3)

diff --git a/extension/src/cli/retry.ts b/extension/src/cli/retry.ts
@@ -1,22 +1,15 @@
+import { UNEXPECTED_ERROR_CODE } from './constants'
+import { MaybeConsoleError } from './error'
 import { delay } from '../util/time'
 import { Logger } from '../common/logger'
 
-const isNonRetryError = (
-  errorMessage: string,
-  nonRetryErrors: string[]
-): boolean => {
-  for (const partialErrorMessage of nonRetryErrors) {
-    if (errorMessage.includes(partialErrorMessage)) {
-      return true
-    }
-  }
-  return false
+const isUnexpectedError = (error: unknown): boolean => {
+  return (error as MaybeConsoleError).exitCode === UNEXPECTED_ERROR_CODE
 }
 
 export const retry = async (
   getNewPromise: () => Promise<string>,
   args: string,
-  nonRetryErrors: string[],
   waitBeforeRetry = 500
 ): Promise<string> => {
   try {
@@ -25,11 +18,11 @@ export const retry = async (
     const errorMessage = (error as Error).message
     Logger.error(`${args} failed with ${errorMessage} retrying...`)
 
-    if (isNonRetryError(errorMessage, nonRetryErrors)) {
+    if (isUnexpectedError(error)) {
       return ''
     }
 
     await delay(waitBeforeRetry)
-    return retry(getNewPromise, args, nonRetryErrors, waitBeforeRetry * 2)
+    return retry(getNewPromise, args, waitBeforeRetry * 2)
   }
 }