Skip to content

Commit a3ac3b4

Browse files
authored
fix(import): reject interrupted GitHub PR responses (#1189)
Co-authored-by: ydflow <314143294+ydflow@users.noreply.github.com>
1 parent 8ab2027 commit a3ac3b4

3 files changed

Lines changed: 126 additions & 1 deletion

File tree

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
2+
import { spawn } from 'node:child_process';
3+
import fs from 'node:fs';
4+
import http from 'node:http';
5+
import os from 'node:os';
6+
import path from 'node:path';
7+
import { fileURLToPath, pathToFileURL } from 'node:url';
8+
9+
const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../..');
10+
const CLI = path.join(ROOT, 'dist/index.js');
11+
const URLS = [41, 42].map((n) => `https://github.com/team/repo/pull/${n}`);
12+
13+
describe('import --from-mr with interrupted GitHub responses', () => {
14+
let sandbox: string;
15+
let preload: string;
16+
17+
beforeAll(() => {
18+
sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-mr-response-'));
19+
const repo = path.join(sandbox, 'repo');
20+
fs.mkdirSync(repo);
21+
fs.writeFileSync(path.join(repo, 'teamai.yaml'), 'team: response-errors\nrepo: https://example.com/team.git\nprovider: git\n');
22+
fs.mkdirSync(path.join(sandbox, '.teamai'));
23+
fs.writeFileSync(path.join(sandbox, '.teamai/config.yaml'), [
24+
'repo:', ` localPath: ${JSON.stringify(repo)}`, ' remote: https://example.com/team.git',
25+
'username: test-user', 'scope: user', '',
26+
].join('\n'));
27+
preload = path.join(sandbox, 'github-fixture.mjs');
28+
// Only replace the external boundaries: the built CLI reads real HTTP
29+
// responses, including IncomingMessage's error after a socket closes early.
30+
fs.writeFileSync(preload, `
31+
import childProcess from 'node:child_process';
32+
import http from 'node:http';
33+
import https from 'node:https';
34+
import { syncBuiltinESMExports } from 'node:module';
35+
const execSync = childProcess.execSync;
36+
childProcess.execSync = (command, ...args) => {
37+
if (command.startsWith('gh pr ')) throw new Error('gh unavailable in fixture');
38+
return execSync(command, ...args);
39+
};
40+
const request = https.request;
41+
https.request = (options, callback) => options.hostname === 'api.github.com'
42+
? http.request({ ...options, protocol: 'http:', hostname: '127.0.0.1', port: process.env.FIXTURE_PORT }, callback)
43+
: request(options, callback);
44+
syncBuiltinESMExports();
45+
`);
46+
});
47+
48+
afterAll(() => { if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true }); });
49+
50+
it.each(['metadata', 'commits', 'diff'])(
51+
'reports an interrupted %s response and continues to the next PR', async (endpoint) => {
52+
const interrupted: string[] = [];
53+
const server = http.createServer((req, res) => {
54+
const stage = req.headers.accept?.includes('.diff') ? 'diff'
55+
: req.url?.includes('/commits?') ? 'commits' : 'metadata';
56+
if (stage === endpoint) {
57+
interrupted.push(req.url!);
58+
res.writeHead(200, { 'Content-Length': '10000' });
59+
res.write('partial response');
60+
setImmediate(() => res.destroy());
61+
return;
62+
}
63+
res.end(JSON.stringify(stage === 'metadata'
64+
? { title: 'Test PR', body: '', user: { login: 'dev' }, merged_at: null }
65+
: [{ sha: 'abc123', commit: { message: 'Test commit' } }]));
66+
});
67+
await new Promise<void>((resolve) => { server.listen(0, '127.0.0.1', resolve); });
68+
try {
69+
const address = server.address();
70+
if (!address || typeof address === 'string') throw new Error('Expected a TCP address');
71+
const result = await new Promise<{ code: number | null; output: string }>((resolve, reject) => {
72+
const child = spawn(process.execPath, [
73+
'--import', pathToFileURL(preload).href, CLI, 'import', '--from-mr', ...URLS,
74+
'--all', '--output', path.join(sandbox, 'output'),
75+
], {
76+
cwd: sandbox,
77+
env: {
78+
...process.env, HOME: sandbox, USERPROFILE: sandbox, FORCE_COLOR: '0',
79+
GITHUB_TOKEN: '', GH_TOKEN: '', FIXTURE_PORT: String(address.port),
80+
},
81+
stdio: ['ignore', 'pipe', 'pipe'],
82+
});
83+
let output = '';
84+
child.stdout.on('data', (chunk: Buffer) => { output += chunk.toString(); });
85+
child.stderr.on('data', (chunk: Buffer) => { output += chunk.toString(); });
86+
const timer = setTimeout(() => { child.kill(); reject(new Error(`CLI timed out: ${output}`)); }, 20_000);
87+
child.on('error', (error) => { clearTimeout(timer); reject(error); });
88+
child.on('close', (code) => { clearTimeout(timer); resolve({ code, output }); });
89+
});
90+
expect(result.code, result.output).toBe(1);
91+
expect(result.output).toContain('Import from MR complete: 0 succeeded, 2 failed');
92+
for (const url of URLS) expect(result.output).toContain(`import from MR failed: ${url}`);
93+
expect(result.output).not.toContain("Unhandled 'error' event");
94+
expect(result.output).not.toContain('AI analysis in progress');
95+
expect(interrupted).toHaveLength(2);
96+
expect(fs.existsSync(path.join(sandbox, 'output'))).toBe(false);
97+
} finally {
98+
server.closeAllConnections();
99+
await new Promise<void>((resolve, reject) => { server.close((error) => error ? reject(error) : resolve()); });
100+
}
101+
},
102+
);
103+
});

‎src/__tests__/github-mr-fetch.test.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { EventEmitter } from 'node:events';
22
import type { IncomingMessage } from 'node:http';
33
import https from 'node:https';
4+
import { setImmediate } from 'node:timers/promises';
45
import { afterEach, describe, expect, it, vi } from 'vitest';
56
import { fetchGitHubPR } from '../providers/github/mr-fetch.js';
67

@@ -12,7 +13,7 @@ vi.mock('node:child_process', async (importOriginal) => ({
1213
const PR = '/repos/team/repo/pulls/42';
1314
const METADATA = { title: 'Fix retries', body: null, user: { login: 'dev' }, merged_at: null };
1415

15-
function mockResponses(failedPath?: string, status = 200, body = ''): string[] {
16+
function mockResponses(failedPath?: string, status = 200, body = '', responseError?: Error): string[] {
1617
const seen: string[] = [];
1718
const request = (options: https.RequestOptions, callback: (res: IncomingMessage) => void) => {
1819
const url = String(options.path);
@@ -31,6 +32,13 @@ function mockResponses(failedPath?: string, status = 200, body = ''): string[] {
3132
const res = Object.assign(new EventEmitter(), { statusCode: failed ? status : 200 });
3233
callback(res as IncomingMessage);
3334
res.emit('data', Buffer.from(response));
35+
if (failed && responseError) {
36+
// Keep the baseline's unhandled event from crashing the test worker.
37+
res.on('error', () => {});
38+
res.emit('error', responseError);
39+
res.emit('close');
40+
return;
41+
}
3442
res.emit('end');
3543
});
3644
},
@@ -44,6 +52,18 @@ function mockResponses(failedPath?: string, status = 200, body = ''): string[] {
4452
afterEach(() => { vi.restoreAllMocks(); });
4553

4654
describe('GitHub PR REST fallback', () => {
55+
it.each([PR, `${PR}/commits?per_page=50`, `${PR}.diff`])(
56+
'rejects when the response stream fails at %s', async (endpoint) => {
57+
const error = new Error('aborted');
58+
const seen = mockResponses(endpoint, 200, 'partial response', error);
59+
await expect(Promise.race([
60+
fetchGitHubPR('https://github.com/team/repo/pull/42'),
61+
setImmediate('response error was not handled'),
62+
])).rejects.toBe(error);
63+
expect(seen.at(-1)).toBe(endpoint.replace(/\.diff$/, ''));
64+
},
65+
);
66+
4767
it.each([
4868
[404, '{"message":"Not Found"}'],
4969
[403, '<html>Rate limit exceeded</html>'],

‎src/providers/github/mr-fetch.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ async function githubApiGet(path: string): Promise<unknown> {
7171
const req = https.request(
7272
{ hostname: 'api.github.com', path, headers },
7373
(res) => {
74+
res.on('error', reject);
7475
const chunks: Buffer[] = [];
7576
res.on('data', (c: Buffer) => chunks.push(c));
7677
res.on('end', () => {
@@ -124,6 +125,7 @@ async function fetchGitHubPRViaApi(owner: string, repo: string, number: string):
124125
const req = https.request(
125126
{ hostname: 'api.github.com', path: `/repos/${owner}/${repo}/pulls/${number}`, headers },
126127
(res) => {
128+
res.on('error', reject);
127129
const chunks: Buffer[] = [];
128130
res.on('data', (c: Buffer) => chunks.push(c));
129131
res.on('end', () => {

0 commit comments

Comments
 (0)