Merge pull request #1445 from DataDog/rodrigo.roca/improve-error-mess…

…age-on-correlate-command

[ci-visibility] Show error message on deployment correlate command

src/commands/deployment/__tests__/correlate.test.ts

@@ -1,6 +1,6 @@
 import {Cli} from 'clipanion/lib/advanced'
 
-import {createMockContext} from '../../../helpers/__tests__/fixtures'
+import {createMockContext, getAxiosError} from '../../../helpers/__tests__/fixtures'
 
 import {DeploymentCorrelateCommand} from '../correlate'
 
@@ -75,4 +75,26 @@ describe('execute', () => {
       "CI_JOB_ID": "1"
     }`)
   })
+  test('handleError', async () => {
+    const command = new DeploymentCorrelateCommand()
+    command['context'] = createMockContext() as any
+
+    const axiosError = getAxiosError(400, {
+      message: 'Request failed with status code 400',
+      errors: ['Some validation error'],
+    })
+
+    command['handleError'](axiosError)
+
+    expect(command['context'].stdout.toString()).toStrictEqual(
+      `[ERROR] Could not send deployment correlation data: {
+  "status": 400,
+  "response": {
+    "errors": [
+      "Some validation error"
+    ]
+  }
+}\n`
+    )
+  })
 })

src/commands/deployment/correlate.ts

@@ -1,3 +1,4 @@
+import {isAxiosError} from 'axios'
 import chalk from 'chalk'
 import {Command, Option} from 'clipanion'
 import simpleGit from 'simple-git'
@@ -148,7 +149,8 @@ export class DeploymentCorrelateCommand extends Command {
         retries: 5,
       })
     } catch (error) {
-      this.logger.error(`Failed to send deployment correlation data: ${error.message}`)
+      // TODO: use `coerceError()`
+      this.handleError(error as Error)
     }
   }
 
@@ -166,4 +168,21 @@ export class DeploymentCorrelateCommand extends Command {
 
     return true
   }
+
+  private handleError(error: Error) {
+    this.context.stderr.write(
+      `${chalk.red.bold('[ERROR]')} Could not send deployment correlation data: ${
+        isAxiosError(error)
+          ? JSON.stringify(
+              {
+                status: error.response?.status,
+                response: error.response?.data as unknown,
+              },
+              undefined,
+              2
+            )
+          : error.message
+      }\n`
+    )
+  }
 }

src/commands/synthetics/__tests__/api.test.ts

@@ -5,6 +5,8 @@ import type {AxiosResponse} from 'axios'
 
 import axios, {AxiosError} from 'axios'
 
+import {getAxiosError} from '../../../helpers/__tests__/fixtures'
+
 import {apiConstructor, formatBackendErrors, getApiHelper} from '../api'
 import {CriticalError} from '../errors'
 import {ExecutionRule, PollResult, ServerResult, TestPayload, Trigger} from '../interfaces'
@@ -14,7 +16,6 @@ import * as utils from '../utils/public'
 import {
   ciConfig,
   getApiTest,
-  getAxiosHttpError,
   getSyntheticsProxy,
   MOBILE_PRESIGNED_URLS_PAYLOAD,
   MOBILE_PRESIGNED_UPLOAD_PARTS,
@@ -478,31 +479,31 @@ describe('getApiHelper', () => {
 
 describe('formatBackendErrors', () => {
   test('backend error - no error', () => {
-    const backendError = getAxiosHttpError(500, {errors: []})
+    const backendError = getAxiosError(500, {errors: []})
     expect(formatBackendErrors(backendError)).toBe('error querying https://app.datadoghq.com/example')
   })
 
   test('backend error - single error', () => {
-    const backendError = getAxiosHttpError(500, {errors: ['single error']})
+    const backendError = getAxiosError(500, {errors: ['single error']})
     expect(formatBackendErrors(backendError)).toBe(
       'query on https://app.datadoghq.com/example returned: "single error"'
     )
   })
 
   test('backend error - multiple errors', () => {
-    const backendError = getAxiosHttpError(500, {errors: ['error 1', 'error 2']})
+    const backendError = getAxiosError(500, {errors: ['error 1', 'error 2']})
     expect(formatBackendErrors(backendError)).toBe(
       'query on https://app.datadoghq.com/example returned:\n  - error 1\n  - error 2'
     )
   })
 
   test('not a backend error', () => {
-    const requestError = getAxiosHttpError(403, {message: 'Forbidden'})
+    const requestError = getAxiosError(403, {message: 'Forbidden'})
     expect(formatBackendErrors(requestError)).toBe('could not query https://app.datadoghq.com/example\nForbidden')
   })
 
   test('missing scope error', () => {
-    const requestError = getAxiosHttpError(403, {errors: ['Forbidden', 'Failed permission authorization checks']})
+    const requestError = getAxiosError(403, {errors: ['Forbidden', 'Failed permission authorization checks']})
     expect(formatBackendErrors(requestError, 'synthetics_default_settings_read')).toBe(
       'query on https://app.datadoghq.com/example returned:\n  - Forbidden\n  - Failed permission authorization checks\nIs the App key granted the synthetics_default_settings_read scope?'
     )

src/commands/synthetics/__tests__/cli.test.ts

@@ -1,6 +1,6 @@
 import {Cli} from 'clipanion/lib/advanced'
 
-import {createCommand} from '../../../helpers/__tests__/fixtures'
+import {createCommand, getAxiosError} from '../../../helpers/__tests__/fixtures'
 import * as ciUtils from '../../../helpers/utils'
 
 import * as api from '../api'
@@ -11,12 +11,12 @@ import {
   UploadApplicationCommandConfig,
   UserConfigOverride,
 } from '../interfaces'
-import {DEFAULT_COMMAND_CONFIG, DEFAULT_POLLING_TIMEOUT, RunTestsCommand} from '../run-tests-command'
+import {DEFAULT_COMMAND_CONFIG, RunTestsCommand} from '../run-tests-command'
 import {DEFAULT_UPLOAD_COMMAND_CONFIG, UploadApplicationCommand} from '../upload-application-command'
 import {toBoolean, toNumber, toExecutionRule, toStringMap} from '../utils/internal'
 import * as utils from '../utils/public'
 
-import {getApiTest, getAxiosHttpError, getTestSuite, mockApi, mockTestTriggerResponse} from './fixtures'
+import {getApiTest, getTestSuite, mockApi, mockTestTriggerResponse} from './fixtures'
 test('all option flags are supported', async () => {
   const options = [
     'apiKey',
@@ -1048,7 +1048,7 @@ describe('run-test', () => {
       }
 
       const triggerTests = jest.fn(() => {
-        throw getAxiosHttpError(502, {message: 'Bad Gateway'})
+        throw getAxiosError(502, {message: 'Bad Gateway'})
       })
       const apiHelper = mockApi({
         getTest: jest.fn(async () => ({...getApiTest('publicId')})),
@@ -1228,7 +1228,7 @@ describe('run-test', () => {
 
       const apiHelper = mockApi({
         getTest: jest.fn(() => {
-          throw getAxiosHttpError(404, {errors: ['Test not found']})
+          throw getAxiosError(404, {errors: ['Test not found']})
         }),
       })
       jest.spyOn(ciUtils, 'resolveConfigFromFile').mockImplementation(async (config, _) => config)
@@ -1251,7 +1251,7 @@ describe('run-test', () => {
 
           const apiHelper = mockApi({
             searchTests: jest.fn(() => {
-              throw errorCode ? getAxiosHttpError(errorCode, {message: 'Error'}) : new Error('Unknown error')
+              throw errorCode ? getAxiosError(errorCode, {message: 'Error'}) : new Error('Unknown error')
             }),
           })
           jest.spyOn(api, 'getApiHelper').mockReturnValue(apiHelper)
@@ -1267,7 +1267,7 @@ describe('run-test', () => {
 
           const apiHelper = mockApi({
             getTest: jest.fn(() => {
-              throw errorCode ? getAxiosHttpError(errorCode, {message: 'Error'}) : new Error('Unknown error')
+              throw errorCode ? getAxiosError(errorCode, {message: 'Error'}) : new Error('Unknown error')
             }),
           })
           jest.spyOn(ciUtils, 'resolveConfigFromFile').mockImplementation(async (config, __) => config)
@@ -1285,7 +1285,7 @@ describe('run-test', () => {
           const apiHelper = mockApi({
             getTest: async () => getApiTest('123-456-789'),
             triggerTests: jest.fn(() => {
-              throw errorCode ? getAxiosHttpError(errorCode, {message: 'Error'}) : new Error('Unknown error')
+              throw errorCode ? getAxiosError(errorCode, {message: 'Error'}) : new Error('Unknown error')
             }),
           })
           jest.spyOn(ciUtils, 'resolveConfigFromFile').mockImplementation(async (config, __) => config)
@@ -1304,7 +1304,7 @@ describe('run-test', () => {
             getBatch: async () => ({results: [], status: 'passed'}),
             getTest: async () => getApiTest('123-456-789'),
             pollResults: jest.fn(() => {
-              throw errorCode ? getAxiosHttpError(errorCode, {message: 'Error'}) : new Error('Unknown error')
+              throw errorCode ? getAxiosError(errorCode, {message: 'Error'}) : new Error('Unknown error')
             }),
             triggerTests: async () => mockTestTriggerResponse,
           })
@@ -1336,7 +1336,7 @@ describe('run-test', () => {
         const apiHelper = mockApi({
           getTest: jest.fn(async (testId: string) => {
             if (testId === 'mis-sin-ggg') {
-              throw getAxiosHttpError(404, {errors: ['Test not found']})
+              throw getAxiosError(404, {errors: ['Test not found']})
             }
 
             return {} as ServerTest
@@ -1369,7 +1369,7 @@ describe('run-test', () => {
       const apiHelper = mockApi({
         getTest: jest.fn(async (testId: string) => {
           if (testId === 'for-bid-den') {
-            const serverError = getAxiosHttpError(403, {errors: ['Forbidden']})
+            const serverError = getAxiosError(403, {errors: ['Forbidden']})
             serverError.config.url = 'tests/for-bid-den'
             throw serverError
           }

src/commands/synthetics/__tests__/fixtures.ts

@@ -2,9 +2,6 @@ import * as http from 'http'
 import * as net from 'net'
 import {URL} from 'url'
 
-import type {AxiosResponse, InternalAxiosRequestConfig} from 'axios'
-
-import {AxiosError} from 'axios'
 import WebSocket, {Server as WebSocketServer} from 'ws'
 
 import {ProxyConfiguration} from '../../../helpers/utils'
@@ -52,8 +49,6 @@ const mockUser: User = {
   name: '',
 }
 
-export const MOCK_BASE_URL = 'https://app.datadoghq.com/'
-
 export type MockedReporter = {
   [K in keyof MainReporter]: jest.Mock<void, Parameters<MainReporter[K]>>
 }
@@ -94,14 +89,6 @@ export const ciConfig: RunTestsCommandConfig = {
   variableStrings: [],
 }
 
-export const getAxiosHttpError = (status: number, {errors, message}: {errors?: string[]; message?: string}) => {
-  const serverError = new AxiosError(message) as AxiosError<any> & {config: InternalAxiosRequestConfig}
-  serverError.config = {baseURL: MOCK_BASE_URL, url: 'example'} as InternalAxiosRequestConfig
-  serverError.response = {data: {errors}, status} as AxiosResponse
-
-  return serverError
-}
-
 export const getApiTest = (publicId = 'abc-def-ghi', opts: Partial<Test> = {}): Test => ({
   config: {
     assertions: [],

src/commands/synthetics/__tests__/reporters/default.test.ts

@@ -2,6 +2,8 @@ jest.unmock('chalk')
 
 import {BaseContext} from 'clipanion/lib/advanced'
 
+import {MOCK_BASE_URL} from '../../../../helpers/__tests__/fixtures'
+
 import {
   ExecutionRule,
   MainReporter,
@@ -23,7 +25,6 @@ import {
   getIncompleteServerResult,
   getSummary,
   getTimedOutBrowserResult,
-  MOCK_BASE_URL,
 } from '../fixtures'
 
 /**

src/commands/synthetics/__tests__/reporters/junit.test.ts

@@ -4,6 +4,8 @@ import {Writable} from 'stream'
 
 import {BaseContext} from 'clipanion/lib/advanced'
 
+import {MOCK_BASE_URL} from '../../../../helpers/__tests__/fixtures'
+
 import {Device, ExecutionRule, Result, Test} from '../../interfaces'
 import {Args, getDefaultSuiteStats, getDefaultTestCaseStats, JUnitReporter, XMLTestCase} from '../../reporters/junit'
 import {RunTestsCommand} from '../../run-tests-command'
@@ -23,7 +25,6 @@ import {
   getMultiStepsServerResult,
   getStep,
   getSummary,
-  MOCK_BASE_URL,
 } from '../fixtures'
 
 const globalTestMock = getApiTest('123-456-789')

src/commands/synthetics/__tests__/run-tests-lib.test.ts

@@ -1,5 +1,6 @@
 import fs from 'fs'
 
+import {getAxiosError} from '../../../helpers/__tests__/fixtures'
 import * as ciUtils from '../../../helpers/utils'
 
 import * as api from '../api'
@@ -16,7 +17,6 @@ import {
   ciConfig,
   getApiResult,
   getApiTest,
-  getAxiosHttpError,
   getMobileTest,
   MOBILE_PRESIGNED_URLS_PAYLOAD,
   mockReporter,
@@ -332,7 +332,7 @@ describe('run-test', () => {
       test(`getTestsList throws - ${status}`, async () => {
         const apiHelper = {
           searchTests: jest.fn(() => {
-            throw getAxiosHttpError(status, {message: 'Server Error'})
+            throw getAxiosError(status, {message: 'Server Error'})
           }),
         }
         jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any)
@@ -348,7 +348,7 @@ describe('run-test', () => {
       test(`getTestsToTrigger throws - ${status}`, async () => {
         const apiHelper = {
           getTest: jest.fn(() => {
-            throw getAxiosHttpError(status, {errors: ['Bad Gateway']})
+            throw getAxiosError(status, {errors: ['Bad Gateway']})
           }),
         }
         jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any)
@@ -378,7 +378,7 @@ describe('run-test', () => {
 
       const apiHelper = {
         getTunnelPresignedURL: jest.fn(() => {
-          throw getAxiosHttpError(502, {message: 'Server Error'})
+          throw getAxiosError(502, {message: 'Server Error'})
         }),
       }
 
@@ -412,7 +412,7 @@ describe('run-test', () => {
 
       const apiHelper = {
         getMobileApplicationPresignedURLs: jest.fn(() => {
-          throw getAxiosHttpError(502, {message: 'Server Error'})
+          throw getAxiosError(502, {message: 'Server Error'})
         }),
       }
 
@@ -449,7 +449,7 @@ describe('run-test', () => {
       const apiHelper = {
         getMobileApplicationPresignedURLs: jest.fn(() => MOBILE_PRESIGNED_URLS_PAYLOAD),
         uploadMobileApplicationPart: jest.fn(() => {
-          throw getAxiosHttpError(502, {message: 'Server Error'})
+          throw getAxiosError(502, {message: 'Server Error'})
         }),
       }
 
@@ -480,7 +480,7 @@ describe('run-test', () => {
       const apiHelper = {
         getTunnelPresignedURL: () => ({url: 'url'}),
         triggerTests: jest.fn(() => {
-          throw getAxiosHttpError(502, {errors: ['Bad Gateway']})
+          throw getAxiosError(502, {errors: ['Bad Gateway']})
         }),
       }
 
@@ -531,7 +531,7 @@ describe('run-test', () => {
         getBatch: () => ({results: []}),
         getTunnelPresignedURL: () => ({url: 'url'}),
         pollResults: jest.fn(() => {
-          throw getAxiosHttpError(502, {errors: ['Bad Gateway']})
+          throw getAxiosError(502, {errors: ['Bad Gateway']})
         }),
       }