Skip to content

Commit b69ecd1

Browse files
leeroybrunleeroybrun
committed
fix(pipeline): handle gh API errors in promote
Add robust error handling to scripts/pipeline/github/promote-branch.mjs: introduce normalizeExecError, isGhNotFoundError, and formatExecError helpers; switch PATCH calls to use typed -F fields; only fall back to creating the ref when the PATCH fails with a 404 (do not mask other errors). Add tests (scripts/release/pipeline_promote_branch_script.test.mjs) that stub the gh CLI to verify typed force usage, ensure no POST fallback on forbidden errors, and validate the overall promote behavior.
1 parent 66ca124 commit b69ecd1

2 files changed

Lines changed: 193 additions & 4 deletions

File tree

‎scripts/pipeline/github/promote-branch.mjs‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,41 @@ function run(cmd, args, opts) {
8181
});
8282
}
8383

84+
/**
85+
* @param {unknown} err
86+
* @returns {{ stdout: string; stderr: string; message: string }}
87+
*/
88+
function normalizeExecError(err) {
89+
const anyErr = /** @type {{ stdout?: unknown; stderr?: unknown; message?: unknown }} */ (err ?? {});
90+
return {
91+
stdout: typeof anyErr?.stdout === 'string' ? anyErr.stdout : '',
92+
stderr: typeof anyErr?.stderr === 'string' ? anyErr.stderr : '',
93+
message: typeof anyErr?.message === 'string' ? anyErr.message : String(err ?? ''),
94+
};
95+
}
96+
97+
/**
98+
* @param {unknown} err
99+
* @returns {boolean}
100+
*/
101+
function isGhNotFoundError(err) {
102+
const { stderr, message } = normalizeExecError(err);
103+
return stderr.includes('(HTTP 404)') || message.includes('(HTTP 404)');
104+
}
105+
106+
/**
107+
* @param {unknown} err
108+
* @returns {string}
109+
*/
110+
function formatExecError(err) {
111+
const { stdout, stderr, message } = normalizeExecError(err);
112+
const parts = [];
113+
if (message) parts.push(message.trim());
114+
if (stderr) parts.push(stderr.trim());
115+
if (stdout) parts.push(stdout.trim());
116+
return parts.filter(Boolean).join('\n');
117+
}
118+
84119
/**
85120
* @param {string[]} files
86121
*/
@@ -225,13 +260,18 @@ function main() {
225260
const force = mode === 'reset';
226261

227262
try {
228-
run('gh', ['api', '-X', 'PATCH', updateApi, '-f', `sha=${sourceSha}`, '-f', `force=${force}`], { env: ghEnv });
229-
} catch {
263+
run('gh', ['api', '-X', 'PATCH', updateApi, '-F', `sha=${sourceSha}`, '-F', `force=${force}`], { env: ghEnv });
264+
} catch (err) {
265+
if (!isGhNotFoundError(err)) {
266+
fail(`Failed to update refs/heads/${target}.\n${formatExecError(err)}`);
267+
}
268+
230269
// If ref doesn't exist yet, create it.
231270
const createApi = `repos/${repo}/git/refs`;
232-
run('gh', ['api', '-X', 'POST', createApi, '-f', `ref=refs/heads/${target}`, '-f', `sha=${sourceSha}`], { env: ghEnv });
271+
run('gh', ['api', '-X', 'POST', createApi, '-f', `ref=refs/heads/${target}`, '-f', `sha=${sourceSha}`], {
272+
env: ghEnv,
273+
});
233274
}
234275
}
235276

236277
main();
237-

‎scripts/release/pipeline_promote_branch_script.test.mjs‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
import test from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import fs from 'node:fs';
4+
import os from 'node:os';
5+
import path from 'node:path';
6+
import { spawnSync } from 'node:child_process';
7+
8+
const repoRoot = path.resolve(import.meta.dirname, '..', '..');
9+
const scriptPath = path.join(repoRoot, 'scripts', 'pipeline', 'github', 'promote-branch.mjs');
10+
11+
function writeExecutable(filePath, content) {
12+
fs.writeFileSync(filePath, content, { encoding: 'utf8', mode: 0o700 });
13+
}
14+
15+
function writeGhStub(binDir) {
16+
const ghPath = path.join(binDir, 'gh');
17+
writeExecutable(
18+
ghPath,
19+
[
20+
'#!/usr/bin/env node',
21+
"import fs from 'node:fs';",
22+
'',
23+
'const logPath = process.env.GH_STUB_LOG;',
24+
'if (logPath) fs.appendFileSync(logPath, `${JSON.stringify(process.argv.slice(2))}\\n`, \"utf8\");',
25+
'',
26+
'const args = process.argv.slice(2);',
27+
"if (args[0] !== 'api') process.exit(0);",
28+
'',
29+
"let method = 'GET';",
30+
'let endpoint = "";',
31+
'const rawFields = [];',
32+
'const typedFields = [];',
33+
'',
34+
'for (let i = 1; i < args.length; i++) {',
35+
' const a = args[i];',
36+
" if (a === '-X' || a === '--method') { method = args[i + 1] ?? method; i++; continue; }",
37+
' if ((a === "-f" || a === "--raw-field") && args[i + 1]) { rawFields.push(args[i + 1]); i++; continue; }',
38+
' if ((a === "-F" || a === "--field") && args[i + 1]) { typedFields.push(args[i + 1]); i++; continue; }',
39+
' if (!endpoint && !a.startsWith("-")) endpoint = a;',
40+
'}',
41+
'',
42+
'function hasTypedForceTrue() {',
43+
' return typedFields.some((f) => f === "force=true");',
44+
'}',
45+
'',
46+
'function write422(message) {',
47+
' process.stdout.write(JSON.stringify({ message, status: "422" }));',
48+
' process.stderr.write(`gh: ${message} (HTTP 422)\\n`);',
49+
' process.exit(1);',
50+
'}',
51+
'',
52+
'function write403(message) {',
53+
' process.stdout.write(JSON.stringify({ message, status: "403" }));',
54+
' process.stderr.write(`gh: ${message} (HTTP 403)\\n`);',
55+
' process.exit(1);',
56+
'}',
57+
'',
58+
'if (method === "GET") {',
59+
' if (endpoint.includes("/git/ref/heads/dev")) { process.stdout.write("SOURCE_SHA\\n"); process.exit(0); }',
60+
' if (endpoint.includes("/git/ref/heads/main")) { process.stdout.write("TARGET_SHA\\n"); process.exit(0); }',
61+
' if (endpoint.includes("/compare/main...dev")) {',
62+
' process.stdout.write(JSON.stringify({ status: "ahead", ahead_by: 1, behind_by: 0, files: [] }));',
63+
' process.exit(0);',
64+
' }',
65+
' process.stdout.write("");',
66+
' process.exit(0);',
67+
'}',
68+
'',
69+
'if (method === "PATCH" && endpoint.includes("/git/refs/heads/main")) {',
70+
' const outcome = process.env.GH_STUB_PATCH_OUTCOME ?? "require_typed_force";',
71+
' if (outcome === "forbidden") write403("Forbidden");',
72+
' if (!hasTypedForceTrue()) write422("Update is not a fast forward");',
73+
' process.exit(0);',
74+
'}',
75+
'',
76+
'if (method === "POST" && endpoint.endsWith("/git/refs")) {',
77+
' const message = process.env.GH_STUB_CREATE_MESSAGE ?? "Reference already exists";',
78+
' write422(message);',
79+
'}',
80+
'',
81+
'process.exit(0);',
82+
'',
83+
].join('\n'),
84+
);
85+
return ghPath;
86+
}
87+
88+
function runPromoteBranch({ patchOutcome }) {
89+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'happier-promote-branch-script-'));
90+
const binDir = path.join(dir, 'bin');
91+
fs.mkdirSync(binDir, { recursive: true });
92+
93+
const logPath = path.join(dir, 'gh.log');
94+
writeGhStub(binDir);
95+
96+
const env = {
97+
...process.env,
98+
PATH: `${binDir}:${process.env.PATH ?? ''}`,
99+
GH_REPO: 'happier-dev/happier',
100+
GH_TOKEN: 'test-token',
101+
GH_STUB_LOG: logPath,
102+
GH_STUB_PATCH_OUTCOME: patchOutcome ?? 'require_typed_force',
103+
};
104+
105+
const res = spawnSync(
106+
process.execPath,
107+
[
108+
scriptPath,
109+
'--source',
110+
'dev',
111+
'--target',
112+
'main',
113+
'--mode',
114+
'reset',
115+
'--allow-reset',
116+
'true',
117+
'--confirm',
118+
'reset main from dev',
119+
],
120+
{ cwd: repoRoot, env, encoding: 'utf8' },
121+
);
122+
123+
const calls = fs
124+
.readFileSync(logPath, 'utf8')
125+
.trim()
126+
.split('\n')
127+
.filter(Boolean)
128+
.map((line) => JSON.parse(line));
129+
130+
return { res, calls };
131+
}
132+
133+
test('promote-branch reset uses typed force update (no fallback create)', () => {
134+
const { res, calls } = runPromoteBranch({ patchOutcome: 'require_typed_force' });
135+
136+
assert.equal(res.status, 0, `expected success (stderr: ${res.stderr})`);
137+
assert.ok(calls.some((c) => c.includes('-X') && c.includes('PATCH')), 'expected PATCH call to update ref');
138+
assert.ok(calls.some((c) => c.includes('-F') && c.includes('force=true')), 'expected typed force=true field');
139+
assert.ok(!calls.some((c) => c.includes('-X') && c.includes('POST')), 'expected no POST fallback create call');
140+
});
141+
142+
test('promote-branch does not mask PATCH failures by attempting create', () => {
143+
const { res, calls } = runPromoteBranch({ patchOutcome: 'forbidden' });
144+
145+
assert.notEqual(res.status, 0, 'expected failure');
146+
assert.match(res.stderr, /\bForbidden\b/);
147+
assert.ok(!calls.some((c) => c.includes('-X') && c.includes('POST')), 'expected no POST fallback create call');
148+
});
149+

0 commit comments

Comments
 (0)