Skip to content

Commit fd514d0

Browse files
committed
Harden avatar fetches: per-avatar error handling, timeout, redirects, concurrency
Addresses review points #6 (per-avatar try/catch so one flaky fetch doesn't break the weekly cron) and #3 (request timeout + redirect handling). - Wrap each fetch in try/catch; fall back to a neutral grey placeholder and log a warning rather than failing the whole job. - 15s request timeout via req.setTimeout. - Follow up to 3 HTTP 301/302/307/308 redirects. - Parallelise fetches with a small concurrency limiter (8 in flight) instead of serial awaits. Made-with: Cursor
1 parent d94526e commit fd514d0

1 file changed

Lines changed: 58 additions & 7 deletions

File tree

.github/workflows/contributors.yml

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,24 @@ jobs:
4444
'&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;',
4545
})[c]);
4646
47-
function fetchDataUri(url) {
47+
const REQUEST_TIMEOUT_MS = 15000;
48+
const MAX_REDIRECTS = 3;
49+
const FETCH_CONCURRENCY = 8;
50+
51+
function fetchDataUri(url, redirectsLeft = MAX_REDIRECTS) {
4852
return new Promise((resolve, reject) => {
4953
const req = https.get(
5054
url,
5155
{ headers: { 'User-Agent': 'causalpy-contributors-workflow' } },
5256
(res) => {
57+
if ([301, 302, 307, 308].includes(res.statusCode) && res.headers.location) {
58+
res.resume();
59+
if (redirectsLeft <= 0) {
60+
return reject(new Error(`too many redirects for ${url}`));
61+
}
62+
const next = new URL(res.headers.location, url).toString();
63+
return resolve(fetchDataUri(next, redirectsLeft - 1));
64+
}
5365
if (res.statusCode !== 200) {
5466
res.resume();
5567
return reject(new Error(`HTTP ${res.statusCode} for ${url}`));
@@ -63,11 +75,41 @@ jobs:
6375
});
6476
},
6577
);
78+
req.setTimeout(REQUEST_TIMEOUT_MS, () => {
79+
req.destroy(new Error(`timeout after ${REQUEST_TIMEOUT_MS}ms for ${url}`));
80+
});
6681
req.on('error', reject);
6782
req.end();
6883
});
6984
}
7085
86+
// Neutral placeholder used when an avatar fetch fails. Keeps the
87+
// layout stable and lets the workflow still open a PR instead of
88+
// failing the whole run on a single flaky request.
89+
const PLACEHOLDER_DATA_URI =
90+
'data:image/svg+xml;base64,' +
91+
Buffer.from(
92+
'<svg xmlns="http://www.w3.org/2000/svg" width="64" height="64">' +
93+
'<rect width="64" height="64" fill="#d0d7de"/>' +
94+
'</svg>',
95+
).toString('base64');
96+
97+
async function mapWithConcurrency(items, limit, worker) {
98+
const results = new Array(items.length);
99+
let next = 0;
100+
async function run() {
101+
while (true) {
102+
const i = next++;
103+
if (i >= items.length) return;
104+
results[i] = await worker(items[i], i);
105+
}
106+
}
107+
await Promise.all(
108+
Array.from({ length: Math.min(limit, items.length) }, run),
109+
);
110+
return results;
111+
}
112+
71113
core.info(`Fetching contributors for ${context.repo.owner}/${context.repo.repo}`);
72114
const contributors = await github.paginate(
73115
github.rest.repos.listContributors,
@@ -82,9 +124,8 @@ jobs:
82124
const width = PAD * 2 + COLS * AVATAR + (COLS - 1) * GAP;
83125
const height = PAD * 2 + rows * AVATAR + (rows - 1) * GAP;
84126
85-
const cells = [];
86-
for (let i = 0; i < people.length; i++) {
87-
const c = people[i];
127+
let failures = 0;
128+
const cells = await mapWithConcurrency(people, FETCH_CONCURRENCY, async (c, i) => {
88129
const col = i % COLS;
89130
const row = Math.floor(i / COLS);
90131
const x = PAD + col * (AVATAR + GAP);
@@ -97,14 +138,24 @@ jobs:
97138
const title = xmlEscape(c.login);
98139
const base = (c.avatar_url || '').split('?')[0];
99140
const sized = `${base}?s=${AVATAR * 2}`;
100-
const dataUri = await fetchDataUri(sized);
101-
cells.push([
141+
let dataUri;
142+
try {
143+
dataUri = await fetchDataUri(sized);
144+
} catch (err) {
145+
failures += 1;
146+
core.warning(`Avatar fetch failed for ${c.login}: ${err.message}; using placeholder`);
147+
dataUri = PLACEHOLDER_DATA_URI;
148+
}
149+
return [
102150
` <a href="${href}" target="_blank" rel="noopener">`,
103151
` <title>${title}</title>`,
104152
` <clipPath id="${clipId}"><circle cx="${cx}" cy="${cy}" r="${r}" /></clipPath>`,
105153
` <image href="${dataUri}" x="${x}" y="${y}" width="${AVATAR}" height="${AVATAR}" clip-path="url(#${clipId})" preserveAspectRatio="xMidYMid slice" />`,
106154
` </a>`,
107-
].join('\n'));
155+
].join('\n');
156+
});
157+
if (failures > 0) {
158+
core.warning(`${failures}/${people.length} avatars used the placeholder`);
108159
}
109160
110161
const svg = [

0 commit comments

Comments
 (0)