Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SYNTH-16108] Show error stack trace and cause #1438

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/__mocks__/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const util = jest.requireActual('util')
const originalInspect = util.inspect

const newInspect = (object, options) => {
return originalInspect(object, {...options, colors: false, depth: options.depth})
return originalInspect(object, {...options, colors: false, depth: options?.depth})
}

Object.assign(newInspect, originalInspect)
Expand Down
4 changes: 3 additions & 1 deletion src/commands/synthetics/__tests__/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,9 @@ describe('run-test', () => {
expect(writeMock).toHaveBeenCalledWith('[aaa-aaa-aaa] Found test "aaa-aaa-aaa"\n')
expect(writeMock).toHaveBeenCalledWith('[bbb-bbb-bbb] Found test "bbb-bbb-bbb" (1 test override)\n')
expect(writeMock).toHaveBeenCalledWith(
'\n ERROR: authorization error \nFailed to get test: query on https://app.datadoghq.com/tests/for-bid-den returned: "Forbidden"\n\n\n'
expect.stringContaining(
'\n ERROR: authorization error \nCriticalError: Failed to get test: query on https://app.datadoghq.com/tests/for-bid-den returned: "Forbidden"'
)
)
expect(writeMock).toHaveBeenCalledWith(
'Credentials refused, make sure `apiKey`, `appKey` and `datadogSite` are correct.\n'
Expand Down
25 changes: 18 additions & 7 deletions src/commands/synthetics/__tests__/run-tests-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ describe('run-test', () => {
test(`getTestsList throws - ${status}`, async () => {
const apiHelper = {
searchTests: jest.fn(() => {
throw getAxiosHttpError(status, {message: 'Server Error'})
throw getAxiosHttpError(status, {errors: [status === 403 ? 'Forbidden' : 'Bad Gateway']})
}),
}
jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any)
Expand All @@ -342,13 +342,22 @@ describe('run-test', () => {
testSearchQuery: 'a-search-query',
tunnel: true,
})
).rejects.toThrow(new CriticalError(error, 'Server Error'))
).rejects.toThrow(
new CriticalError(
error,
status === 403
? 'Failed to search tests with query: query on https://app.datadoghq.com/example returned: "Forbidden"'
: new Error(
'Failed to search tests with query: query on https://app.datadoghq.com/example returned: "Bad Gateway"'
)
)
)
})

test(`getTestsToTrigger throws - ${status}`, async () => {
const apiHelper = {
getTest: jest.fn(() => {
throw getAxiosHttpError(status, {errors: ['Bad Gateway']})
throw getAxiosHttpError(status, {errors: [status === 403 ? 'Forbidden' : 'Bad Gateway']})
}),
}
jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any)
Expand All @@ -361,7 +370,9 @@ describe('run-test', () => {
).rejects.toThrow(
new CriticalError(
error,
'Failed to get test: query on https://app.datadoghq.com/example returned: "Bad Gateway"\n'
status === 403
? 'Failed to get test: query on https://app.datadoghq.com/example returned: "Forbidden"'
: new Error('Failed to get test: query on https://app.datadoghq.com/example returned: "Bad Gateway"')
)
)
})
Expand Down Expand Up @@ -389,7 +400,7 @@ describe('run-test', () => {
publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'],
tunnel: true,
})
).rejects.toThrow(new CriticalError('UNAVAILABLE_TUNNEL_CONFIG', 'Server Error'))
).rejects.toThrow(new CriticalError('UNAVAILABLE_TUNNEL_CONFIG', new Error('Server Error')))
})

test.each(compat)('getMobileApplicationPresignedURLs throws ($compat)', async ({defaultTestOverrides}) => {
Expand Down Expand Up @@ -494,7 +505,7 @@ describe('run-test', () => {
).rejects.toThrow(
new CriticalError(
'TRIGGER_TESTS_FAILED',
'[] Failed to trigger tests: query on https://app.datadoghq.com/example returned: "Bad Gateway"\n'
new Error('[] Failed to trigger tests: query on https://app.datadoghq.com/example returned: "Bad Gateway"')
)
)
expect(stopTunnelSpy).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -546,7 +557,7 @@ describe('run-test', () => {
).rejects.toThrow(
new CriticalError(
'POLL_RESULTS_FAILED',
'Failed to poll results: query on https://app.datadoghq.com/example returned: "Bad Gateway"\n'
new Error('Failed to poll results: query on https://app.datadoghq.com/example returned: "Bad Gateway"')
)
)
expect(stopTunnelSpy).toHaveBeenCalledTimes(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ exports[`utils reportCiError should report AUTHORIZATION_ERROR error 1`] = `
[
"
 ERROR: authorization error 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'AUTHORIZATION_ERROR',
[cause]: undefined
}

",
],
Expand All @@ -45,7 +50,12 @@ exports[`utils reportCiError should report INVALID_CONFIG error 1`] = `
[
"
 ERROR: invalid config 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'INVALID_CONFIG',
[cause]: undefined
}

",
],
Expand Down Expand Up @@ -118,7 +128,7 @@ exports[`utils reportCiError should report NO_TESTS_TO_RUN error 1`] = `
"calls": [
[
"
 ERROR: No tests to run 
 ERROR: no tests to run 


",
Expand All @@ -139,7 +149,12 @@ exports[`utils reportCiError should report POLL_RESULTS_FAILED error 1`] = `
[
"
 ERROR: unable to poll test results 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'POLL_RESULTS_FAILED',
[cause]: undefined
}

",
],
Expand All @@ -159,7 +174,12 @@ exports[`utils reportCiError should report TOO_MANY_TESTS_TO_TRIGGER error 1`] =
[
"
 ERROR: too many tests to trigger 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'TOO_MANY_TESTS_TO_TRIGGER',
[cause]: undefined
}

",
],
Expand All @@ -179,7 +199,12 @@ exports[`utils reportCiError should report TRIGGER_TESTS_FAILED error 1`] = `
[
"
 ERROR: unable to trigger tests 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'TRIGGER_TESTS_FAILED',
[cause]: undefined
}

",
],
Expand All @@ -199,7 +224,12 @@ exports[`utils reportCiError should report TUNNEL_START_FAILED error 1`] = `
[
"
 ERROR: unable to start tunnel 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'TUNNEL_START_FAILED',
[cause]: undefined
}

",
],
Expand All @@ -218,8 +248,13 @@ exports[`utils reportCiError should report UNAVAILABLE_TEST_CONFIG error 1`] = `
"calls": [
[
"
 ERROR: unable to obtain test configurations with search query 

 ERROR: unable to obtain test configurations 
CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'UNAVAILABLE_TEST_CONFIG',
[cause]: undefined
}

",
],
Expand All @@ -239,7 +274,12 @@ exports[`utils reportCiError should report UNAVAILABLE_TUNNEL_CONFIG error 1`] =
[
"
 ERROR: unable to get tunnel configuration 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'UNAVAILABLE_TUNNEL_CONFIG',
[cause]: undefined
}

",
],
Expand All @@ -259,7 +299,12 @@ exports[`utils reportCiError should report default Error if no CiError was match
[
"
 ERROR 

CiError:
at someFunction (someFile.js:42:42)
at someFunction (someFile.js:42:42) {
code: 'ERROR',
[cause]: undefined
}

",
],
Expand Down
24 changes: 21 additions & 3 deletions src/commands/synthetics/__tests__/utils/public.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jest.mock('path', () => {
import child_process from 'child_process'
import * as fs from 'fs'
import process from 'process'
import util from 'util'

import type * as path from 'path'

Expand Down Expand Up @@ -450,7 +451,7 @@ describe('utils', () => {

await expect(() =>
utils.getTestAndOverrideConfig(api, triggerConfig, mockReporter, getSummary())
).rejects.toThrow('Failed to get test: could not query https://app.datadoghq.com/example\nForbidden\n')
).rejects.toThrow('Failed to get test: could not query https://app.datadoghq.com/example\nForbidden')
})

test('Passes when public ID is valid', async () => {
Expand Down Expand Up @@ -1731,7 +1732,7 @@ describe('utils', () => {
mockReporter
)
).rejects.toThrow(
'Failed to poll results: could not query https://app.datadoghq.com/example\nPoll results server error\n'
'Failed to poll results: could not query https://app.datadoghq.com/example\nPoll results server error'
)

expect(pollResultsMock).toHaveBeenCalledWith([result.resultId])
Expand All @@ -1757,7 +1758,7 @@ describe('utils', () => {
mockReporter
)
).rejects.toThrow(
'Failed to get batch: could not query https://app.datadoghq.com/example\nGet batch server error\n'
'Failed to get batch: could not query https://app.datadoghq.com/example\nGet batch server error'
)

expect(getBatchMock).toHaveBeenCalledWith(trigger.batch_id)
Expand Down Expand Up @@ -2158,6 +2159,23 @@ describe('utils', () => {
})

describe('reportCiError', () => {
const originalInspect = jest.requireActual('util').inspect
beforeEach(() => {
jest.spyOn(util, 'inspect').mockImplementation((error) => {
error.stack = `Error: ${error.message}\n at someFunction (someFile.js:42:42)\n at someFunction (someFile.js:42:42)`

// Node 14 doesn't show the `[cause]` in the stack trace as shown here: https://nodejs.org/api/errors.html#errorcause
// So we mock it here to align the unit tests' snapshots across all Node.js versions on which we run Jest in the CI.
if (process.version.startsWith('v14')) {
error.cause = undefined

return originalInspect(error).replace('cause: undefined', '[cause]: undefined')
}

return originalInspect(error)
})
})

test.each([
'NO_TESTS_TO_RUN',
'MISSING_TESTS',
Expand Down
2 changes: 1 addition & 1 deletion src/commands/synthetics/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export const determineRetryDelay = (
}
}

const isEndpointError = (error: Error): error is EndpointError => error instanceof EndpointError
export const isEndpointError = (error: Error): error is EndpointError => error instanceof EndpointError

const getErrorHttpStatus = (error: Error): number | undefined =>
isEndpointError(error) ? error.status : isAxiosError(error) ? error.response?.status : undefined
Expand Down
4 changes: 2 additions & 2 deletions src/commands/synthetics/batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ const getBatch = async (api: APIHelper, batchId: string): Promise<Batch> => {

return batch
} catch (e) {
throw new EndpointError(`Failed to get batch: ${formatBackendErrors(e)}\n`, e.response?.status)
throw new EndpointError(`Failed to get batch: ${formatBackendErrors(e)}`, e.response?.status)
}
}

Expand All @@ -285,7 +285,7 @@ const getPollResultMap = async (api: APIHelper, resultIds: string[]) => {

return {pollResultMap, incompleteResultIds}
} catch (e) {
throw new EndpointError(`Failed to poll results: ${formatBackendErrors(e)}\n`, e.response?.status)
throw new EndpointError(`Failed to poll results: ${formatBackendErrors(e)}`, e.response?.status)
}
}

Expand Down
50 changes: 46 additions & 4 deletions src/commands/synthetics/errors.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
/* eslint-disable max-classes-per-file */

import {isAxiosError} from 'axios'

import {coerceError} from '../../helpers/errors'

import {isEndpointError, isForbiddenError} from './api'

const nonCriticalErrorCodes = ['NO_TESTS_TO_RUN', 'MISSING_TESTS'] as const
export type NonCriticalCiErrorCode = typeof nonCriticalErrorCodes[number]

Expand All @@ -22,14 +30,15 @@ const criticalErrorCodes = [
'INVALID_MOBILE_APP_UPLOAD_PARAMETERS',
'MOBILE_APP_UPLOAD_TIMEOUT',
'UNKNOWN_MOBILE_APP_UPLOAD_FAILURE',
'UNKNOWN',
] as const
export type CriticalCiErrorCode = typeof criticalErrorCodes[number]

export type CiErrorCode = NonCriticalCiErrorCode | CriticalCiErrorCode

export class CiError extends Error {
constructor(public code: CiErrorCode, message?: string) {
super(message)
constructor(public code: CiErrorCode, message?: string, cause?: Error) {
super(message, {cause})
}

public toJson() {
Expand All @@ -41,8 +50,11 @@ export class CiError extends Error {
}

export class CriticalError extends CiError {
constructor(public code: CriticalCiErrorCode, message?: string) {
super(code, message)
constructor(public code: CriticalCiErrorCode, cause?: string | Error) {
const message = typeof cause === 'string' ? cause : cause?.message
const error = cause instanceof Error ? cause : undefined

super(code, message, error)
}
}

Expand All @@ -51,3 +63,33 @@ export class BatchTimeoutRunawayError extends CriticalError {
super('BATCH_TIMEOUT_RUNAWAY', "The batch didn't timeout after the expected timeout period.")
}
}

export const wrapError = (e: unknown): Error => {
const error = coerceError(e)
if (error instanceof CiError) {
return error
}

if (isAxiosError(error)) {
// Avoid leaking any unexpected information.
delete error.config
delete error.request
delete error.response

if (isForbiddenError(error)) {
return new CriticalError('AUTHORIZATION_ERROR', error.message)
}

return error
}

if (isForbiddenError(error)) {
return new CriticalError('AUTHORIZATION_ERROR', error.message)
}

if (isEndpointError(error)) {
return error
}

return new CriticalError('UNKNOWN', error)
}
Loading