Skip to content

Commit 1ec32f8

Browse files
Ishani-PrajapatiSecuredByIshani
andauthored
Fix/10505 overlapping api requests (#11084)
* fix: correct malformed .heatmap-grid CSS declaration * fix: replace unsafe innerHTML rendering with safe DOM APIs * fix: prevent overlapping GitHub profile requests --------- Co-authored-by: SecuredByIshani <ishani.prajapati.2006@email.com>
1 parent 0ae3a4f commit 1ec32f8

1 file changed

Lines changed: 156 additions & 26 deletions

File tree

‎public/github_profile_finder/script.js‎

Lines changed: 156 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,50 @@ class DataCacheEngine {
104104
}
105105
}
106106

107+
/* =========================================================
108+
REQUEST LIFECYCLE (Bug #10505)
109+
========================================================= */
110+
111+
// Only one profile search or comparison may be "in flight" at a
112+
// time. currentOperationController lets us abort whatever the
113+
// previous one was doing the moment a new one starts, and
114+
// currentOperationId is a generation counter so that async work
115+
// which can't be aborted (e.g. a fetch that already resolved, or
116+
// a loop that's mid-iteration) can still recognize it has become
117+
// stale and refuse to touch the UI.
118+
let currentOperationController = null;
119+
let currentOperationId = 0;
120+
121+
// Call at the start of every profile search / comparison. Aborts
122+
// the previous operation (if any) so its in-flight requests stop
123+
// and its results can never overwrite what the user just asked
124+
// for, then hands back a fresh signal + id for the new operation.
125+
function beginOperation() {
126+
if (currentOperationController) {
127+
currentOperationController.abort();
128+
}
129+
currentOperationController = new AbortController();
130+
currentOperationId += 1;
131+
132+
return {
133+
signal: currentOperationController.signal,
134+
operationId: currentOperationId
135+
};
136+
}
137+
138+
// True once a newer operation has started, meaning this one's
139+
// results are stale and must not be rendered.
140+
function isStaleOperation(operationId) {
141+
return operationId !== currentOperationId;
142+
}
143+
144+
// An AbortError means a request was cancelled on purpose (a newer
145+
// search/comparison took over) - it is not a real failure and
146+
// must never surface as an error banner to the user.
147+
function isAbortError(error) {
148+
return Boolean(error) && error.name === "AbortError";
149+
}
150+
107151
/* =========================================================
108152
UTILITIES
109153
========================================================= */
@@ -309,9 +353,12 @@ function sleep(ms) {
309353

310354
/**
311355
* Fetch wrapper with a timeout, since a hung request should
312-
* never leave the heatmap stuck on "Loading...".
356+
* never leave the heatmap stuck on "Loading...". Also accepts the
357+
* outer search/comparison operation's signal (externalSignal) so
358+
* that cancelling the whole operation aborts this request
359+
* immediately too, instead of waiting out its own timeout.
313360
*/
314-
async function fetchWithTimeout(url, timeoutMs) {
361+
async function fetchWithTimeout(url, timeoutMs, externalSignal) {
315362

316363
const controller = new AbortController();
317364

@@ -320,13 +367,28 @@ async function fetchWithTimeout(url, timeoutMs) {
320367
timeoutMs
321368
);
322369

370+
const abortFromOutside = () => controller.abort();
371+
372+
if (externalSignal) {
373+
374+
if (externalSignal.aborted) {
375+
controller.abort();
376+
} else {
377+
externalSignal.addEventListener("abort", abortFromOutside);
378+
}
379+
}
380+
323381
try {
324382

325383
return await fetch(url, { signal: controller.signal });
326384

327385
} finally {
328386

329387
clearTimeout(timer);
388+
389+
if (externalSignal) {
390+
externalSignal.removeEventListener("abort", abortFromOutside);
391+
}
330392
}
331393
}
332394

@@ -337,11 +399,12 @@ async function fetchWithTimeout(url, timeoutMs) {
337399
* { date, count, level } day objects. Only used when
338400
* GRAPHQL_PROXY_ENDPOINT is configured.
339401
*/
340-
async function fetchContributionsFromGraphQLProxy(username) {
402+
async function fetchContributionsFromGraphQLProxy(username, signal) {
341403

342404
const response = await fetchWithTimeout(
343405
`${GRAPHQL_PROXY_ENDPOINT}?username=${encodeURIComponent(username)}`,
344-
CONTRIBUTIONS_FETCH_TIMEOUT_MS
406+
CONTRIBUTIONS_FETCH_TIMEOUT_MS,
407+
signal
345408
);
346409

347410
if (!response.ok) {
@@ -371,7 +434,7 @@ async function fetchContributionsFromGraphQLProxy(username) {
371434
* fabricated data - any unrecoverable failure is thrown so the
372435
* caller can show an honest error state.
373436
*/
374-
async function fetchContributionsFromFallbackApi(username) {
437+
async function fetchContributionsFromFallbackApi(username, signal) {
375438

376439
let lastError = null;
377440

@@ -381,7 +444,8 @@ async function fetchContributionsFromFallbackApi(username) {
381444

382445
const response = await fetchWithTimeout(
383446
`${CONTRIBUTIONS_FALLBACK_API_URL}/${username}?y=last`,
384-
CONTRIBUTIONS_FETCH_TIMEOUT_MS
447+
CONTRIBUTIONS_FETCH_TIMEOUT_MS,
448+
signal
385449
);
386450

387451
if (response.status === 404) {
@@ -430,19 +494,28 @@ async function fetchContributionsFromFallbackApi(username) {
430494
const isAbort = error.name === "AbortError";
431495
const isNotFound = error.status === 404;
432496

433-
// Don't retry on a bad username or an aborted/timed-out
434-
// request that's already exhausted its own budget once;
435-
// only retry genuinely transient failures.
497+
// If the *outer* search/comparison was cancelled (a newer one
498+
// started), this AbortError is an intentional cancellation,
499+
// not a transient failure - retrying it would just re-issue
500+
// work nobody wants anymore. Only genuine timeouts (this
501+
// request's own budget expiring) are retryable.
502+
const isExternalCancel = Boolean(signal && signal.aborted);
503+
504+
// Don't retry on a bad username, an intentionally cancelled
505+
// operation, or an aborted/timed-out request that's already
506+
// exhausted its own budget once; only retry genuinely
507+
// transient failures.
436508
const isRetryable =
437509
!isNotFound &&
510+
!isExternalCancel &&
438511
(isAbort ||
439512
error.status === 429 ||
440513
error.status >= 500 ||
441514
error.status === undefined);
442515

443516
if (!isRetryable || attempt === CONTRIBUTIONS_MAX_RETRIES) {
444517

445-
if (isAbort) {
518+
if (isAbort && !isExternalCancel) {
446519

447520
lastError = new Error(
448521
"The request timed out while loading contribution activity."
@@ -469,7 +542,7 @@ async function fetchContributionsFromFallbackApi(username) {
469542
* falls back to the public read-only contributions API. Never
470543
* falls back to randomly generated or fake data.
471544
*/
472-
async function fetchContributionData(username) {
545+
async function fetchContributionData(username, signal) {
473546

474547
const cacheKey = `contributions_${username}`;
475548

@@ -478,8 +551,8 @@ async function fetchContributionData(username) {
478551
if (cached) return cached;
479552

480553
const contributions = GRAPHQL_PROXY_ENDPOINT
481-
? await fetchContributionsFromGraphQLProxy(username)
482-
: await fetchContributionsFromFallbackApi(username);
554+
? await fetchContributionsFromGraphQLProxy(username, signal)
555+
: await fetchContributionsFromFallbackApi(username, signal);
483556

484557
DataCacheEngine.set(cacheKey, contributions);
485558

@@ -560,7 +633,7 @@ function showHeatmapError(message) {
560633
UI.heatmapGrid.appendChild(errorNode);
561634
}
562635

563-
async function generateContributionHeatmap(username) {
636+
async function generateContributionHeatmap(username, signal, operationId) {
564637

565638
if (!UI.heatmapGrid) return;
566639

@@ -579,12 +652,21 @@ async function generateContributionHeatmap(username) {
579652
try {
580653

581654
const contributions =
582-
await fetchContributionData(username);
655+
await fetchContributionData(username, signal);
656+
657+
// A newer search/comparison may have started while this was
658+
// in flight; ignore the now-stale result instead of rendering
659+
// over whatever the newer operation has already shown.
660+
if (isStaleOperation(operationId)) return;
583661

584662
renderContributionHeatmap(contributions);
585663

586664
} catch (error) {
587665

666+
// Cancellation (this operation or the whole page's operation
667+
// being superseded) is expected, not an error - stay silent.
668+
if (isAbortError(error) || isStaleOperation(operationId)) return;
669+
588670
showHeatmapError(getContributionErrorMessage(error));
589671
}
590672
}
@@ -626,7 +708,7 @@ function getContributionErrorMessage(error) {
626708
LANGUAGE ANALYTICS
627709
========================================================= */
628710

629-
async function renderLanguageAnalytics(repos) {
711+
async function renderLanguageAnalytics(repos, signal, operationId) {
630712

631713
if (!repos || !repos.length) return;
632714

@@ -639,7 +721,7 @@ async function renderLanguageAnalytics(repos) {
639721
if (!repo.languages_url) continue;
640722

641723
const response =
642-
await fetch(repo.languages_url);
724+
await fetch(repo.languages_url, { signal });
643725

644726
if (!response.ok) continue;
645727

@@ -662,6 +744,11 @@ async function renderLanguageAnalytics(repos) {
662744

663745
if (!totalBytes) return;
664746

747+
// A newer search/comparison may have started while these
748+
// per-repo language requests were in flight; don't let a
749+
// stale result overwrite the current UI.
750+
if (isStaleOperation(operationId)) return;
751+
665752
const sortedLanguages =
666753
Object.entries(languageBytes)
667754
.sort((a, b) => b[1] - a[1])
@@ -754,6 +841,10 @@ async function renderLanguageAnalytics(repos) {
754841

755842
} catch (error) {
756843

844+
// Cancellation is expected here (a newer operation took over)
845+
// and isn't a real failure worth logging.
846+
if (isAbortError(error)) return;
847+
757848
console.error(
758849
"Language analytics failed:",
759850
error
@@ -1001,12 +1092,18 @@ async function fetchUser(username) {
10011092
return;
10021093
}
10031094

1095+
// Cancel any previous profile search/comparison so it can't
1096+
// finish late and clobber the UI with stale data (Bug #10505),
1097+
// then run this one under its own signal + generation id.
1098+
const { signal, operationId } = beginOperation();
1099+
10041100
showLoading();
10051101

10061102
try {
10071103

10081104
const userResponse = await fetch(
1009-
`https://api.github.com/users/${cleanName}`
1105+
`https://api.github.com/users/${cleanName}`,
1106+
{ signal }
10101107
);
10111108

10121109
if (!userResponse.ok) {
@@ -1020,7 +1117,8 @@ async function fetchUser(username) {
10201117
await userResponse.json();
10211118

10221119
const repoResponse = await fetch(
1023-
`https://api.github.com/users/${cleanName}/repos?per_page=100`
1120+
`https://api.github.com/users/${cleanName}/repos?per_page=100`,
1121+
{ signal }
10241122
);
10251123

10261124
if (!repoResponse.ok) {
@@ -1051,13 +1149,22 @@ async function fetchUser(username) {
10511149
)
10521150
.slice(0, 6);
10531151

1152+
// A newer search/comparison may have started while the above
1153+
// requests were in flight; if so, this result is stale and
1154+
// must not be rendered over the newer one.
1155+
if (isStaleOperation(operationId)) return;
1156+
10541157
renderProfile(user);
10551158

10561159
renderRepos(sortedRepos);
10571160

1058-
await generateContributionHeatmap(cleanName);
1161+
await generateContributionHeatmap(cleanName, signal, operationId);
1162+
1163+
if (isStaleOperation(operationId)) return;
1164+
1165+
await renderLanguageAnalytics(repos, signal, operationId);
10591166

1060-
await renderLanguageAnalytics(repos);
1167+
if (isStaleOperation(operationId)) return;
10611168

10621169
UI.analyticsPanel?.classList.remove(
10631170
"hidden"
@@ -1067,6 +1174,12 @@ async function fetchUser(username) {
10671174

10681175
} catch (error) {
10691176

1177+
// An aborted request means a newer search/comparison started
1178+
// and cancelled this one on purpose - that's not a failure, so
1179+
// don't show an error banner or reset UI the newer operation
1180+
// may already own.
1181+
if (isAbortError(error) || isStaleOperation(operationId)) return;
1182+
10701183
resetProfileUI();
10711184

10721185
showStatus(
@@ -1080,7 +1193,7 @@ async function fetchUser(username) {
10801193
FETCH PROFILE DATA
10811194
========================================================= */
10821195

1083-
async function fetchProfileData(username) {
1196+
async function fetchProfileData(username, signal) {
10841197

10851198
const cleanName =
10861199
username.trim().replace("@", "");
@@ -1104,7 +1217,8 @@ async function fetchProfileData(username) {
11041217
}
11051218

11061219
const userResponse = await fetch(
1107-
`https://api.github.com/users/${cleanName}`
1220+
`https://api.github.com/users/${cleanName}`,
1221+
{ signal }
11081222
);
11091223

11101224
if (!userResponse.ok) {
@@ -1118,7 +1232,8 @@ async function fetchProfileData(username) {
11181232
await userResponse.json();
11191233

11201234
const repoResponse = await fetch(
1121-
`https://api.github.com/users/${cleanName}/repos?per_page=50`
1235+
`https://api.github.com/users/${cleanName}/repos?per_page=50`,
1236+
{ signal }
11221237
);
11231238

11241239
if (!repoResponse.ok) {
@@ -1418,6 +1533,10 @@ if (UI.compareForm) {
14181533
return;
14191534
}
14201535

1536+
// Cancel any previous profile search/comparison so it can't
1537+
// finish late and clobber this comparison's UI (Bug #10505).
1538+
const { signal, operationId } = beginOperation();
1539+
14211540
try {
14221541

14231542
showCompareLoading();
@@ -1428,15 +1547,21 @@ if (UI.compareForm) {
14281547
] = await Promise.all([
14291548

14301549
fetchProfileData(
1431-
leftUsername
1550+
leftUsername,
1551+
signal
14321552
),
14331553

14341554
fetchProfileData(
1435-
rightUsername
1555+
rightUsername,
1556+
signal
14361557
)
14371558

14381559
]);
14391560

1561+
// A newer search/comparison may have started while these
1562+
// requests were in flight; ignore this now-stale result.
1563+
if (isStaleOperation(operationId)) return;
1564+
14401565
renderComparison(
14411566
leftData,
14421567
rightData
@@ -1446,6 +1571,11 @@ if (UI.compareForm) {
14461571

14471572
} catch (error) {
14481573

1574+
// An aborted request means a newer search/comparison
1575+
// started and cancelled this one on purpose - not a real
1576+
// failure, so stay silent rather than show an error banner.
1577+
if (isAbortError(error) || isStaleOperation(operationId)) return;
1578+
14491579
showStatus(
14501580
error.message,
14511581
"error"

0 commit comments

Comments
 (0)