diff --git a/__tests__/buildx/bake.test.ts b/__tests__/buildx/bake.test.ts index 3243a37c..2c8342f9 100644 --- a/__tests__/buildx/bake.test.ts +++ b/__tests__/buildx/bake.test.ts @@ -21,7 +21,9 @@ import path from 'path'; import * as rimraf from 'rimraf'; import {Bake} from '../../src/buildx/bake.js'; +import {Buildx} from '../../src/buildx/buildx.js'; import {Context} from '../../src/context.js'; +import {Exec} from '../../src/exec.js'; import {ExecOptions} from '@actions/exec'; import {BakeDefinition} from '../../src/types/buildx/bake.js'; @@ -72,6 +74,20 @@ describe('resolveWarnings', () => { }); describe('getDefinition', () => { + it('reports the error summary before trailing help text', async () => { + const execSpy = vi.spyOn(Exec, 'getExecOutput').mockResolvedValueOnce({ + exitCode: 1, + stdout: '', + stderr: 'ERROR: failed to parse definition\nLearn more at https://docs.docker.com/\n' + }); + try { + const bake = new Bake({buildx: new Buildx({standalone: true})}); + await expect(bake.getDefinition({})).rejects.toThrow('cannot parse bake definitions: failed to parse definition'); + } finally { + execSpy.mockRestore(); + } + }); + // prettier-ignore test.each([ [ diff --git a/__tests__/cosign/cosign.test.ts b/__tests__/cosign/cosign.test.ts index 9ffb059d..6dac7b0f 100644 --- a/__tests__/cosign/cosign.test.ts +++ b/__tests__/cosign/cosign.test.ts @@ -54,6 +54,21 @@ describe('version', () => { }); }); +describe('getErrorMessage', () => { + test.each([ + {name: 'empty output', stderr: '', expected: 'unknown error'}, + {name: 'whitespace-only output', stderr: ' \r\n\t\n', expected: 'unknown error'}, + {name: 'trailing blank lines', stderr: 'warning\n signing failed \n \t\n', expected: 'signing failed'}, + {name: 'CRLF output', stderr: 'warning\r\nverification failed\r\n', expected: 'verification failed'}, + {name: 'carriage returns', stderr: 'progress\rverification failed\r', expected: 'verification failed'}, + {name: 'terminal formatting', stderr: 'warning\n\u001b[31msigning failed\u001b[0m\n', expected: 'signing failed'}, + {name: 'formatting-only output', stderr: '\u001b[0m\n', expected: 'unknown error'}, + {name: 'execution error', stderr: 'Error: signing failed\n2026/09/11 12:00:00 error during command execution: signing failed\n', expected: '2026/09/11 12:00:00 error during command execution: signing failed'} + ])('$name', ({stderr, expected}) => { + expect(Cosign.getErrorMessage(stderr)).toBe(expected); + }); +}); + describe('versionSatisfies', () => { test.each([ ['v0.4.1', '>=0.3.2', true], diff --git a/__tests__/docker/docker.test.ts b/__tests__/docker/docker.test.ts index 46a406ee..0de37ba6 100644 --- a/__tests__/docker/docker.test.ts +++ b/__tests__/docker/docker.test.ts @@ -167,6 +167,23 @@ describe('getExecOutput', () => { }); }); +describe('getErrorMessage', () => { + it.each([ + {name: 'empty output', stderr: '', expected: 'unknown error'}, + {name: 'whitespace-only output', stderr: ' \r\n\t\n', expected: 'unknown error'}, + {name: 'daemon error', stderr: 'Error response from daemon: pull access denied\n', expected: 'Error response from daemon: pull access denied'}, + {name: 'unprefixed error', stderr: 'invalid reference format\n', expected: 'invalid reference format'}, + {name: 'trailing blank lines', stderr: 'warning\n failed to save image: permission denied \n \t\n', expected: 'failed to save image: permission denied'}, + {name: 'CRLF output', stderr: 'warning\r\ninvalid tar header\r\n', expected: 'invalid tar header'}, + {name: 'carriage returns', stderr: 'progress\rinvalid tar header\r', expected: 'invalid tar header'}, + {name: 'terminal formatting', stderr: 'warning\n\u001b[31minvalid tar header\u001b[0m\n', expected: 'invalid tar header'}, + {name: 'formatting-only output', stderr: '\u001b[0m\n', expected: 'unknown error'}, + {name: 'no special ERROR prefix handling', stderr: 'ERROR: earlier message\ninvalid tar header\n', expected: 'invalid tar header'} + ])('$name', ({stderr, expected}) => { + expect(Docker.getErrorMessage(stderr)).toBe(expected); + }); +}); + describe('pull', () => { const originalDockerConfig = process.env.DOCKER_CONFIG; @@ -194,7 +211,7 @@ describe('pull', () => { ) .mockResolvedValueOnce(execOutput(1, '', 'Error response from daemon: Head "https://registry-1.docker.io/v2/tonistiigi/binfmt/manifests/latest": EOF')) .mockResolvedValueOnce(execOutput(1, '', 'Error response from daemon: received unexpected HTTP status: 503 Service Unavailable')) - .mockResolvedValueOnce(execOutput(1, '', 'Error response from daemon: connection reset by peer')) + .mockResolvedValueOnce(execOutput(1, '', '\u001b[31mError response from daemon: connection reset by peer\u001b[0m\r\n \t\r\n')) .mockResolvedValueOnce(execOutput(0, 'latest: Pulling from tonistiigi/binfmt', '')); const pull = Docker.pull('tonistiigi/binfmt'); @@ -209,6 +226,12 @@ describe('pull', () => { expect(execSpy).toHaveBeenCalledTimes(1); }); + it('reports a clean pull error with trailing blank lines', async () => { + const execSpy = vi.spyOn(Docker, 'getExecOutput').mockResolvedValue(execOutput(1, '', '\u001b[31mError response from daemon: pull access denied\u001b[0m\r\n \t\r\n')); + await expect(Docker.pull('doesnotexist:foo')).rejects.toThrow(new Error('Error response from daemon: pull access denied')); + expect(execSpy).toHaveBeenCalledTimes(1); + }); + it('does not retry rate limit errors', async () => { const execSpy = vi.spyOn(Docker, 'getExecOutput').mockResolvedValue(execOutput(1, '', 'Error response from daemon: toomanyrequests: You have reached your pull rate limit')); await expect(Docker.pull('busybox')).rejects.toThrow('toomanyrequests'); diff --git a/src/buildx/bake.ts b/src/buildx/bake.ts index 8828a6dd..2eaa3ff2 100644 --- a/src/buildx/bake.ts +++ b/src/buildx/bake.ts @@ -171,7 +171,7 @@ export class Bake { const printCmd = await this.buildx.getCommand([...args, '--print', ...(cmdOpts.targets || [])]); return await Exec.getExecOutput(printCmd.command, printCmd.args, execOptions).then(res => { if (res.stderr.length > 0 && res.exitCode != 0) { - throw new Error(`cannot parse bake definitions: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); + throw new Error(`cannot parse bake definitions: ${Buildx.getErrorMessage(res.stderr)}`); } return Bake.parseDefinition(res.stdout.trim()); }); diff --git a/src/buildx/install.ts b/src/buildx/install.ts index 85697a58..7fd34a89 100644 --- a/src/buildx/install.ts +++ b/src/buildx/install.ts @@ -143,7 +143,7 @@ export class Install { ignoreReturnCode: true }).then(res => { if (res.stderr.length > 0 && res.exitCode != 0) { - throw new Error(`build failed with: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); + throw new Error(`build failed with: ${Buildx.getErrorMessage(res.stderr)}`); } return `${outputDir}/buildx`; }); diff --git a/src/cosign/cosign.ts b/src/cosign/cosign.ts index 43998fc1..56e6941e 100644 --- a/src/cosign/cosign.ts +++ b/src/cosign/cosign.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import {stripVTControlCharacters} from 'util'; import * as core from '@actions/core'; import {BUNDLE_V03_MEDIA_TYPE, SerializedBundle} from '@sigstore/bundle'; @@ -92,6 +93,17 @@ export class Cosign { }); } + public static getErrorMessage(stderr: string): string { + const lines = stripVTControlCharacters(stderr).split(/[\r\n]/); + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i].trim(); + if (line) { + return line; + } + } + return 'unknown error'; + } + public async versionSatisfies(range: string, version?: string): Promise { const ver = version ?? (await this.version()); if (!ver) { diff --git a/src/cosign/install.ts b/src/cosign/install.ts index 47f45316..00cb0c82 100644 --- a/src/cosign/install.ts +++ b/src/cosign/install.ts @@ -130,7 +130,7 @@ export class Install { input: Buffer.from(dockerfileContent) }).then(res => { if (res.stderr.length > 0 && res.exitCode != 0) { - throw new Error(`build failed with: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); + throw new Error(`build failed with: ${Buildx.getErrorMessage(res.stderr)}`); } return `${outputDir}/cosign`; }); diff --git a/src/docker/docker.ts b/src/docker/docker.ts index 4ee6ca13..c7c6a26b 100644 --- a/src/docker/docker.ts +++ b/src/docker/docker.ts @@ -17,6 +17,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; +import {stripVTControlCharacters} from 'util'; import retry from 'async-retry'; import * as core from '@actions/core'; import {ExecOptions, ExecOutput} from '@actions/exec'; @@ -74,6 +75,17 @@ export class Docker { return Exec.getExecOutput('docker', args, Docker.execOptions(options)); } + public static getErrorMessage(stderr: string): string { + const lines = stripVTControlCharacters(stderr).split(/[\r\n]/); + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i].trim(); + if (line) { + return line; + } + } + return 'unknown error'; + } + private static execOptions(options?: ExecOptions): ExecOptions { if (!options) { options = {}; @@ -179,7 +191,7 @@ export class Docker { ignoreReturnCode: true }).then(res => { if (res.stderr.length > 0 && res.exitCode != 0) { - core.warning(`Failed to load image from cache: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); + core.warning(`Failed to load image from cache: ${Docker.getErrorMessage(res.stderr)}`); } }); } @@ -203,7 +215,7 @@ export class Docker { ignoreReturnCode: true }).then(async res => { if (res.stderr.length > 0 && res.exitCode != 0) { - core.warning(`Failed to save image: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); + core.warning(`Failed to save image: ${Docker.getErrorMessage(res.stderr)}`); } else { const cachePath = await imageCache.save(imageTarPath); core.info(`Image cached to ${cachePath}`); @@ -220,7 +232,7 @@ export class Docker { ignoreReturnCode: true }); if (res.stderr.length > 0 && res.exitCode != 0) { - const err = res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'; + const err = Docker.getErrorMessage(res.stderr); if (!Docker.isPullTransientError(err)) { bail(new Error(err)); return; diff --git a/src/sigstore/sigstore.ts b/src/sigstore/sigstore.ts index b32b633a..effbdee9 100644 --- a/src/sigstore/sigstore.ts +++ b/src/sigstore/sigstore.ts @@ -116,8 +116,7 @@ export class Sigstore { const errorMessages = signResult.errors.map(e => `- [${e.code}] ${e.message} : ${e.detail}`).join('\n'); throw new Error(`Cosign sign command failed with errors:\n${errorMessages}`); } else { - // prettier-ignore - throw new Error(`Cosign sign command failed with: ${execRes.stderr.trim().split(/\r?\n/).filter(line => line.length > 0).pop() ?? 'unknown error'}`); + throw new Error(`Cosign sign command failed with: ${Cosign.getErrorMessage(execRes.stderr)}`); } } const parsedBundle = Sigstore.parseBundle(bundleFromJSON(signResult.bundle)); @@ -207,8 +206,7 @@ export class Sigstore { }) as {[key: string]: string} }); if (execRes.exitCode !== 0) { - // prettier-ignore - throw new Error(`Cosign verify command failed with: ${execRes.stderr.trim().split(/\r?\n/).filter(line => line.length > 0).pop() ?? 'unknown error'}`); + throw new Error(`Cosign verify command failed with: ${Cosign.getErrorMessage(execRes.stderr)}`); } const verifyResult = Cosign.parseCommandOutput(execRes.stderr.trim()); return { @@ -245,8 +243,7 @@ export class Sigstore { throw lastError; } } else { - // prettier-ignore - throw new Error(`Cosign verify command failed with: ${execRes.stderr.trim().split(/\r?\n/).filter(line => line.length > 0).pop() ?? 'unknown error'}`); + throw new Error(`Cosign verify command failed with: ${Cosign.getErrorMessage(execRes.stderr)}`); } } } @@ -308,8 +305,7 @@ export class Sigstore { const errorMessages = signResult.errors.map(e => `- [${e.code}] ${e.message} : ${e.detail}`).join('\n'); throw new Error(`Cosign attest-blob command failed with errors:\n${errorMessages}`); } else { - // prettier-ignore - throw new Error(`Cosign attest-blob command failed with: ${execRes.stderr.trim().split(/\r?\n/).filter(line => line.length > 0).pop() ?? 'unknown error'}`); + throw new Error(`Cosign attest-blob command failed with: ${Cosign.getErrorMessage(execRes.stderr)}`); } } const parsedBundle = Sigstore.parseBundle(bundleFromJSON(JSON.parse(fs.readFileSync(bundlePath, {encoding: 'utf-8'}))));