Add retry logic for dependency graph submission
CI-validate-wrappers / validation (push) Has been cancelled
CI-validate-wrappers / validation (push) Has been cancelled
The dependency graph submission to GitHub's Dependency Submission API had no retry logic, causing intermittent HttpError failures that succeed on manual re-run. This adds: - Retry loop with exponential backoff (3 attempts: 1s, 2s, 4s delays) for transient errors (HTTP 429, 5xx) - Non-retryable errors (4xx except 429) fail immediately - HTTP status code included in error messages for better diagnostics - Unit tests for retry classification and error status helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -204,7 +204,8 @@ async function submitDependencyGraphs(dependencyGraphFiles: string[]): Promise<v
|
||||
|
||||
function translateErrorMessage(jsonFile: string, error: Error): string {
|
||||
const relativeJsonFile = getRelativePathFromWorkspace(jsonFile)
|
||||
const mainWarning = `Dependency submission failed for ${relativeJsonFile}.\n${error.message}`
|
||||
const statusInfo = getErrorStatusText(error)
|
||||
const mainWarning = `Dependency submission failed for ${relativeJsonFile}${statusInfo}.\n${error.message}`
|
||||
if (error.message === 'Resource not accessible by integration') {
|
||||
return `${mainWarning}
|
||||
Please ensure that the 'contents: write' permission is available for the workflow job.
|
||||
@@ -214,6 +215,9 @@ Note that this permission is never available for a 'pull_request' trigger from a
|
||||
return mainWarning
|
||||
}
|
||||
|
||||
const DEPENDENCY_SUBMISSION_MAX_ATTEMPTS = 3
|
||||
const DEPENDENCY_SUBMISSION_BASE_DELAY_MS = 1000
|
||||
|
||||
async function submitDependencyGraphFile(jsonFile: string): Promise<void> {
|
||||
const octokit = getOctokit()
|
||||
const jsonContent = fs.readFileSync(jsonFile, 'utf8')
|
||||
@@ -221,10 +225,42 @@ async function submitDependencyGraphFile(jsonFile: string): Promise<void> {
|
||||
const jsonObject = JSON.parse(jsonContent)
|
||||
jsonObject.owner = github.context.repo.owner
|
||||
jsonObject.repo = github.context.repo.repo
|
||||
const response = await octokit.request('POST /repos/{owner}/{repo}/dependency-graph/snapshots', jsonObject)
|
||||
|
||||
const relativeJsonFile = getRelativePathFromWorkspace(jsonFile)
|
||||
core.notice(`Submitted ${relativeJsonFile}: ${response.data.message}`)
|
||||
for (let attempt = 1; attempt <= DEPENDENCY_SUBMISSION_MAX_ATTEMPTS; attempt++) {
|
||||
try {
|
||||
const response = await octokit.request('POST /repos/{owner}/{repo}/dependency-graph/snapshots', jsonObject)
|
||||
const relativeJsonFile = getRelativePathFromWorkspace(jsonFile)
|
||||
core.notice(`Submitted ${relativeJsonFile}: ${response.data.message}`)
|
||||
return
|
||||
} catch (error) {
|
||||
if (isRetryableError(error) && attempt < DEPENDENCY_SUBMISSION_MAX_ATTEMPTS) {
|
||||
const delay = DEPENDENCY_SUBMISSION_BASE_DELAY_MS * Math.pow(2, attempt - 1)
|
||||
core.info(
|
||||
`Dependency submission attempt ${attempt} failed` +
|
||||
`${getErrorStatusText(error)}. ` +
|
||||
`Retrying in ${delay}ms...`
|
||||
)
|
||||
await new Promise(resolve => setTimeout(resolve, delay))
|
||||
} else {
|
||||
throw error
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function isRetryableError(error: unknown): boolean {
|
||||
if (error instanceof Error && 'status' in error) {
|
||||
const status = (error as Error & {status: number}).status
|
||||
return status === 429 || status >= 500
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
export function getErrorStatusText(error: unknown): string {
|
||||
if (error instanceof Error && 'status' in error) {
|
||||
return ` (HTTP ${(error as Error & {status: number}).status})`
|
||||
}
|
||||
return ''
|
||||
}
|
||||
function getReportDirectory(): string {
|
||||
return process.env.DEPENDENCY_GRAPH_REPORT_DIR!
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import {describe, expect, it} from '@jest/globals'
|
||||
|
||||
import {DependencyGraphConfig} from "../../src/configuration"
|
||||
import {isRetryableError, getErrorStatusText} from "../../src/dependency-graph"
|
||||
|
||||
describe('dependency-graph', () => {
|
||||
describe('constructs job correlator', () => {
|
||||
@@ -33,4 +34,68 @@ describe('dependency-graph', () => {
|
||||
expect(id).toBe('workflow-jobid-windows-211-value_with_comma')
|
||||
})
|
||||
})
|
||||
|
||||
describe('isRetryableError', () => {
|
||||
function httpError(status: number, message: string): Error {
|
||||
const error = new Error(message)
|
||||
error.name = 'HttpError'
|
||||
;(error as Error & {status: number}).status = status
|
||||
return error
|
||||
}
|
||||
|
||||
it('returns true for HTTP 429 (rate limit)', () => {
|
||||
expect(isRetryableError(httpError(429, 'rate limit exceeded'))).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for HTTP 500 (internal server error)', () => {
|
||||
expect(isRetryableError(httpError(500, 'Internal Server Error'))).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for HTTP 502 (bad gateway)', () => {
|
||||
expect(isRetryableError(httpError(502, 'Bad Gateway'))).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for HTTP 503 (service unavailable)', () => {
|
||||
expect(isRetryableError(httpError(503, 'Service Unavailable'))).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false for HTTP 403 (forbidden)', () => {
|
||||
expect(isRetryableError(httpError(403, 'Resource not accessible by integration'))).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for HTTP 404 (not found)', () => {
|
||||
expect(isRetryableError(httpError(404, 'Not Found'))).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for HTTP 422 (unprocessable entity)', () => {
|
||||
expect(isRetryableError(httpError(422, 'Validation Failed'))).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for a plain Error without status', () => {
|
||||
expect(isRetryableError(new Error('network error'))).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for non-Error values', () => {
|
||||
expect(isRetryableError('some string')).toBe(false)
|
||||
expect(isRetryableError(null)).toBe(false)
|
||||
expect(isRetryableError(undefined)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getErrorStatusText', () => {
|
||||
it('returns status text for HttpError with status', () => {
|
||||
const error = new Error('Server Error')
|
||||
;(error as Error & {status: number}).status = 503
|
||||
expect(getErrorStatusText(error)).toBe(' (HTTP 503)')
|
||||
})
|
||||
|
||||
it('returns empty string for plain Error without status', () => {
|
||||
expect(getErrorStatusText(new Error('no status'))).toBe('')
|
||||
})
|
||||
|
||||
it('returns empty string for non-Error values', () => {
|
||||
expect(getErrorStatusText('string')).toBe('')
|
||||
expect(getErrorStatusText(null)).toBe('')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user