fix: contain upstream failures and back off repeated retries

src/core/GitHubUtils.ts

@@ -1,3 +1,4 @@
+import { refreshLegacyToken } from "./legacy-token-refresh";
 import AnonymizedRepositoryModel from "./model/anonymizedRepositories/anonymizedRepositories.model";
 import { isConnected } from "../server/database";
 import { githubQuotaKey, githubTokenContext } from "./github-token-context";
@@ -8,7 +9,7 @@ import { createClient, RedisClientType } from "redis";
 
 import AnonymousError from "./AnonymousError";
 import Repository from "./Repository";
-import { getCredential, replaceCredential, getCredentialToken } from "./credentials";
+import { getCredential, getCredentialToken } from "./credentials";
 import config from "../config";
 import { createLogger } from "./logger";
 import { measureStage } from "./request-monitoring";
@@ -337,56 +338,11 @@ async function resolveRepositoryToken(repository: Repository) {
       credential?.persisted &&
       (!tokenAge || tokenAge < new Date(Date.now() - 1000 * 60 * 60 * 24 * 7))
     ) {
-      const url = `https://api.github.com/applications/${config.CLIENT_ID}/token`;
-      const headers = {
-        Accept: "application/vnd.github+json",
-        "X-GitHub-Api-Version": "2022-11-28",
-      };
-
-      const res = await fetch(url, {
-        method: "PATCH",
-        body: JSON.stringify({
-          access_token: ownerAccessToken,
-        }),
-        credentials: "include",
-        headers: {
-          ...headers,
-          Authorization:
-            "Basic " +
-            Buffer.from(
-              config.CLIENT_ID + ":" + config.CLIENT_SECRET
-            ).toString("base64"),
-        },
-      });
-      // Only persist a refreshed token if GitHub actually returned a
-      // valid one. Without this guard, a 4xx/5xx error body (revoked
-      // OAuth, rate limit, transient outage) silently overwrites the
-      // user's stored token with `undefined`, which then propagates as
-      // `Authorization: token undefined` to every subsequent API call —
-      // 401 even on public repos, and the config.GITHUB_TOKEN fallback
-      // below is unreachable because the token field is no longer falsy.
-      if (res.ok) {
-        const resBody = (await res.json().catch(() => null)) as
-          | { token?: unknown }
-          | null;
-        const refreshed =
-          resBody && typeof resBody.token === "string" && resBody.token.length > 0
-            ? resBody.token
-            : null;
-        if (refreshed) {
-          if (await replaceCredential(repository.owner.id, ownerAccessToken, refreshed)) {
-            checkedRepositoryTokens.set(repository, refreshed);
-            return refreshed;
-          }
-          return (await getCredentialToken(repository.owner.id)) || config.GITHUB_TOKEN;
-        }
+      const refreshed = await refreshLegacyToken(repository.owner.id, ownerAccessToken);
+      if (refreshed !== null) {
+        checkedRepositoryTokens.set(repository, refreshed);
+        return refreshed;
       }
-      logger.warn("token refresh failed; falling back", {
-        code: "token_refresh_failed",
-        httpStatus: res.status,
-        username: repository.owner.model.username,
-      });
-      // fall through to the checkToken path / config.GITHUB_TOKEN
     }
     const check = await checkToken(ownerAccessToken);
     if (check) {

src/core/expiring-map.ts

@@ -0,0 +1,17 @@
+/** A bounded cache for short-lived failures. Keys must include access and content versions. */
+export class ExpiringMap<T> {
+  private entries = new Map<string, { value: T; expires: number }>();
+  constructor(private capacity: number, private now = () => Date.now()) {}
+  get(key: string): T | undefined {
+    const entry = this.entries.get(key);
+    if (!entry) return undefined;
+    if (entry.expires <= this.now()) { this.entries.delete(key); return undefined; }
+    return entry.value;
+  }
+  set(key: string, value: T, ttl: number) {
+    for (const [key, entry] of this.entries) if (entry.expires <= this.now()) this.entries.delete(key);
+    this.entries.delete(key);
+    if (this.entries.size >= this.capacity) this.entries.delete(this.entries.keys().next().value!);
+    this.entries.set(key, { value, expires: this.now() + ttl });
+  }
+}

src/core/legacy-token-refresh.ts

@@ -0,0 +1,52 @@
+import { createHash } from "crypto";
+import config from "../config";
+import { getCredentialToken, replaceCredential } from "./credentials";
+import { ExpiringMap } from "./expiring-map";
+import { createLogger } from "./logger";
+
+const logger = createLogger("github");
+const cooldowns = new ExpiringMap<boolean>(1024);
+const pending = new Map<string, Promise<string | null>>();
+
+/** Only legacy OAuth tokens belong to the OAuth application's reset endpoint. */
+export async function refreshLegacyToken(ownerId: string, previous: string): Promise<string | null> {
+  if (!config.GITHUB_OAUTH_ENABLED || !config.CLIENT_ID || !config.CLIENT_SECRET ||
+    config.CLIENT_ID === "CLIENT_ID" || config.CLIENT_SECRET === "CLIENT_SECRET" ||
+    /^(gh[psur]_|github_pat_)/.test(previous)) return null;
+  const key = createHash("sha256").update(JSON.stringify([ownerId, previous, config.CLIENT_ID, config.CLIENT_SECRET])).digest("hex");
+  if (cooldowns.get(key)) return null;
+  const running = pending.get(key);
+  if (running) return running;
+  if (pending.size >= 1024) return null;
+  const refresh = async () => {
+    let status = 502;
+    try {
+      const res = await fetch(`https://api.github.com/applications/${encodeURIComponent(config.CLIENT_ID)}/token`, {
+        method: "PATCH", body: JSON.stringify({ access_token: previous }),
+        signal: AbortSignal.timeout(5000),
+        headers: { Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28",
+          Authorization: "Basic " + Buffer.from(config.CLIENT_ID + ":" + config.CLIENT_SECRET).toString("base64") },
+      });
+      status = res.status;
+      if (res.ok) {
+        const body = await res.json().catch(() => null) as { token?: unknown } | null;
+        if (typeof body?.token === "string" && body.token.length) {
+          if (await replaceCredential(ownerId, previous, body.token)) return body.token;
+          return (await getCredentialToken(ownerId)) || config.GITHUB_TOKEN;
+        }
+      } else {
+        // Release the response connection even when only the status matters.
+        await res.body?.cancel();
+      }
+    } catch {
+      // The existing token can still work during a transient reset-endpoint failure.
+    }
+    cooldowns.set(key, true, status === 401 || status === 404 ? 30 * 60_000 : 60_000);
+    logger.warn("token refresh failed; backing off", { code: "token_refresh_failed", httpStatus: status });
+    return null;
+  };
+  const work = refresh();
+  pending.set(key, work);
+  try { return await work; }
+  finally { if (pending.get(key) === work) pending.delete(key); }
+}

src/core/process-monitoring.ts

@@ -0,0 +1,11 @@
+import { createLogger, serializeError } from "./logger";
+
+/** Observe fatal errors without overriding Node's default exit behavior. */
+export function installFatalErrorLogging(service: "api" | "streamer") {
+  const logger = createLogger("process");
+  const report = (error: Error, origin: string) => logger.error("process fatal error", {
+    ...serializeError(error), code: "process_fatal_error", service, origin,
+  });
+  process.on("uncaughtExceptionMonitor", report);
+  return () => process.removeListener("uncaughtExceptionMonitor", report);
+}

src/core/repository-failure-status.ts

@@ -0,0 +1,11 @@
+/** Persisted repository failures need the same client status as the original error. */
+export function repositoryFailureStatus(code?: string): number {
+  switch (code) {
+    case "repo_not_found": case "branch_not_found": case "commit_not_found": return 404;
+    case "repo_not_accessible": case "repository_not_accessible": return 403;
+    case "token_expired": case "not_connected": return 401;
+    case "github_rate_limit_exceeded": return 429;
+    case "github_unavailable": return 502;
+    default: return 500;
+  }
+}

src/core/source/GitHubStream.ts

@@ -1,3 +1,4 @@
+import { ExpiringMap } from "../expiring-map";
 import { contentGenerationPrefix, contentRetirementMarker } from "../content-generation";
 import { createHash } from "crypto";
 import { coordinatedFill } from "../cache-coordination";
@@ -29,6 +30,7 @@ const logger = createLogger("gh-stream");
 
 const GH_API_CONCURRENCY = 6;
 const cacheFills = new Map<string, Promise<void>>();
+const missingContent = new ExpiringMap<boolean>(2048);
 
 async function pMap<T, R>(
   items: T[],
@@ -64,6 +66,15 @@ export function githubRawFileUrl(
   )}/raw/${encodeURIComponent(commit)}/${encodedPath}`;
 }
 
+function downloadError(error: Error, filePath: string): AnonymousError {
+  if (error instanceof AnonymousError) return error;
+  const response = error as Error & { response?: { statusCode?: number }; status?: number; httpStatus?: number };
+  const status = response.response?.statusCode ?? response.status ?? response.httpStatus;
+  const code = status === 401 || status === 403 ? "file_not_accessible"
+    : status === 404 ? "file_not_found" : status === 422 ? "file_too_big" : "upstream_error";
+  return new AnonymousError(code, { httpStatus: Number.isInteger(status) && status! >= 400 && status! < 500 ? status : 502, cause: error, object: filePath });
+}
+
 export default class GitHubStream extends GitHubBase {
   type: "GitHubDownload" | "GitHubStream" | "Zip" = "GitHubStream";
 
@@ -123,6 +134,17 @@ export default class GitHubStream extends GitHubBase {
     });
   }
 
+  private deferredRawDownload(token: string, filePath: string): stream.Readable {
+    let active: stream.Readable | undefined;
+    const download = () => this.downloadFileViaRaw(token, filePath);
+    const output = stream.Readable.from((async function* () {
+      active = download();
+      for await (const chunk of active) yield chunk;
+    })(), { objectMode: false });
+    output.once("close", () => active?.destroy());
+    return output;
+  }
+
   // Try the blob API, then fall back to the raw URL on statuses where the
   // path-based endpoint can still succeed. 422 is the blob endpoint's size
   // cap; 404 can happen with stale/invalid blob SHAs while the path still
@@ -135,9 +157,9 @@ export default class GitHubStream extends GitHubBase {
     // Public raw downloads need no bearer token and do not consume the
     // unauthenticated REST API quota. GitHub also resolves LFS pointers here.
     if (!token || githubTokenContext(token)?.publicRepository) {
-      return Promise.resolve(this.downloadFileViaRaw(token, filePath));
+      return Promise.resolve(this.deferredRawDownload(token, filePath));
     }
-    return new Promise<stream.Readable>((resolve) => {
+    return new Promise<stream.Readable>((resolve, reject) => {
       const blobStream = this.downloadFile(token, sha);
       let settled = false;
 
@@ -148,7 +170,7 @@ export default class GitHubStream extends GitHubBase {
           filePath,
           statusCode,
         });
-        resolve(this.downloadFileViaRaw(token, filePath));
+        resolve(this.deferredRawDownload(token, filePath));
       };
 
       blobStream.on("error", (err) => {
@@ -160,13 +182,10 @@ export default class GitHubStream extends GitHubBase {
           fallbackToRaw(statusCode);
           return;
         }
-        // Other errors: let the normal pipeline handle them.
-        // Defer destroy so callers can attach error listeners before
-        // the error event fires, avoiding an uncaughtException crash.
+        // Reject before handing a stream to the caller. nextTick runs before
+        // await continuations, so emitting on a returned stream here can crash.
         settled = true;
-        const passthrough = new stream.PassThrough();
-        resolve(passthrough);
-        process.nextTick(() => passthrough.destroy(err));
+        reject(err);
       });
 
       blobStream.on("response", (response) => {
@@ -191,9 +210,7 @@ export default class GitHubStream extends GitHubBase {
     token: string,
     filePath: string
   ): stream.Readable {
-    const out = new stream.PassThrough();
     let active = blobStream;
-    out.once("close", () => active.destroy());
     // The generator only reads upstream when the output has capacity.
     const downloadRaw = () => this.downloadFileViaRaw(token, filePath);
     async function* resolve() {
@@ -225,12 +242,10 @@ export default class GitHubStream extends GitHubBase {
         }
       }
     }
-    const resolved = stream.Readable.from(resolve());
-    out.once("close", () => resolved.destroy());
-    resolved.on("error", (error) => out.destroy(error));
-    resolved.pipe(out);
-
-    return out;
+    const resolved = stream.Readable.from(resolve(), { objectMode: false });
+    // Keep the generator lazy until the consumer has installed its handlers.
+    resolved.once("close", () => active.destroy());
+    return resolved;
   }
 
   async getFileContentCache(
@@ -299,34 +314,38 @@ export default class GitHubStream extends GitHubBase {
       filling = coordinatedFill(key, cached, async () => {
         await assertActive();
         const token = await this.data.getToken();
-        return measureStage("upstream", async () => {
-          const content = await this.downloadWithFallback(token, expected.sha, filePath);
-          let count = 0;
-          const limited = new stream.Transform({ transform(chunk, encoding, callback) {
-            count += chunk.length;
-            callback(count > config.MAX_FILE_SIZE ? new AnonymousError("file_too_big", { httpStatus: 413 }) : null, chunk);
-          } });
-          content.on("error", error => {
-            if (error instanceof AnonymousError) { limited.destroy(error); return; }
-            const status = (error as { response?: { statusCode?: number }; status?: number; httpStatus?: number });
-            const httpStatus = status.response?.statusCode ?? status.status ?? status.httpStatus;
-            const code = httpStatus === 403 ? "file_not_accessible"
-              : httpStatus === 404 ? "file_not_found"
-              : httpStatus === 422 ? "file_too_big" : "upstream_error";
-            limited.destroy(new AnonymousError(code, { httpStatus: httpStatus || 502, cause: error, object: filePath }));
+        const missKey = createHash("sha256").update(JSON.stringify([
+          key, this.data.organization, this.data.repoName, token,
+        ])).digest("hex");
+        if (missingContent.get(missKey)) throw new AnonymousError("file_not_found", { httpStatus: 404 });
+        try {
+          return await measureStage("upstream", async () => {
+            const content = await this.downloadWithFallback(token, expected.sha, filePath)
+              .catch(error => { throw downloadError(error, filePath); });
+            let count = 0;
+            const limited = new stream.Transform({ transform(chunk, encoding, callback) {
+              count += chunk.length;
+              callback(count > config.MAX_FILE_SIZE ? new AnonymousError("file_too_big", { httpStatus: 413 }) : null, chunk);
+            } });
+            content.on("error", error => limited.destroy(downloadError(error, filePath)));
+            limited.once("close", () => content.destroy());
+            limited.on("error", () => {});
+            content.pipe(limited);
+            const deadline = setTimeout(() => limited.destroy(new AnonymousError("upstream_error", { httpStatus: 502 })), 120_000);
+            try {
+              await storage.write(repoId, cachePath, limited, this.type, expected.size);
+              try { await assertActive(); }
+              catch (error) { await storage.rm(repoId, cachePath); throw error; }
+            }
+            finally { clearTimeout(deadline); limited.destroy(); content.destroy(); }
+            return true;
           });
-          limited.once("close", () => content.destroy());
-          limited.on("error", () => {});
-          content.pipe(limited);
-          const deadline = setTimeout(() => limited.destroy(new AnonymousError("upstream_error", { httpStatus: 502 })), 120_000);
-          try {
-            await storage.write(repoId, cachePath, limited, this.type, expected.size);
-            try { await assertActive(); }
-            catch (error) { await storage.rm(repoId, cachePath); throw error; }
+        } catch (error) {
+          if (error instanceof AnonymousError && error.message === "file_not_found" && error.httpStatus === 404) {
+            missingContent.set(missKey, true, 30_000);
           }
-          finally { clearTimeout(deadline); limited.destroy(); content.destroy(); }
-          return true;
-        });
+          throw error;
+        }
       }).then(() => {});
       cacheFills.set(key, filling);
       void filling.finally(() => { if (cacheFills.get(key) === filling) cacheFills.delete(key); }).catch(() => {});

src/server/index.ts

@@ -1,3 +1,5 @@
+import { sendRequestRateLimit } from "./request-rate-limit";
+import { installFatalErrorLogging } from "../core/process-monitoring";
 import { githubAppRouter, githubAppWebhook } from "./routes/github-app";
 import { config as dotenv } from "dotenv";
 dotenv();
@@ -47,6 +49,7 @@ import { createReviewConsentPage, isReviewConsentPagePath } from "./review-conse
 
 const reviewConsentPage = createReviewConsentPage();
 const logger = createLogger("server");
+installFatalErrorLogging("api");
 
 // Lazily build the templated index.html on first request so the server
 // works even when started before `gulp` finishes.
@@ -191,9 +194,7 @@ export default async function start() {
     keyGenerator: requestRateLimitKey,
     standardHeaders: "draft-6",
     legacyHeaders: false,
-    message: (_request: express.Request, _response: express.Response) => {
-      return `You can only make ${config.RATE_LIMIT} requests every 15min. Please try again later.`;
-    },
+    handler: sendRequestRateLimit,
   });
   const speedLimiter = slowDown({
     windowMs: 15 * 60 * 1000, // 15 minutes

src/server/request-rate-limit.ts

@@ -0,0 +1,9 @@
+import { Request, Response } from "express";
+
+/** Give clients a retry time instead of an unparseable plain-text failure. */
+export function sendRequestRateLimit(req: Request, res: Response) {
+  const rate = (req as Request & { rateLimit?: { resetTime?: Date } }).rateLimit;
+  const resetAt = rate?.resetTime?.getTime() || Date.now() + 15 * 60_000;
+  res.setHeader("Retry-After", String(Math.max(1, Math.ceil((resetAt - Date.now()) / 1000))));
+  res.status(429).json({ error: "rate_limited", resetAt });
+}

src/server/routes/repository-public.ts

@@ -1,3 +1,4 @@
+import { repositoryFailureStatus } from "../../core/repository-failure-status";
 import { anonymizePath } from "../../core/anonymize-utils";
 import * as express from "express";
 import config from "../../config";
@@ -334,7 +335,7 @@ router.get(
                 : "repository_not_accessible",
               {
                 object: repo,
-                httpStatus: 500,
+                httpStatus: repositoryFailureStatus(repo.model.statusMessage),
               }
             );
           }

src/server/routes/route-utils.ts

@@ -233,10 +233,10 @@ export function handleError(
     errorCode = String(error);
   }
   let status = 500;
-  if (error.httpStatus) {
-    status = error.httpStatus;
-  } else if (error.$metadata?.httpStatusCode) {
-    status = error.$metadata.httpStatusCode;
+  const explicitStatus = [error?.httpStatus, error?.status, error?.response?.statusCode,
+    error?.$metadata?.httpStatusCode].find(value => Number.isInteger(value) && value >= 400 && value <= 599);
+  if (explicitStatus !== undefined) {
+    status = explicitStatus;
   } else if (
     errorCode &&
     (errorCode.indexOf("not_found") > -1 ||

src/streamer/index.ts

@@ -1,3 +1,4 @@
+import { installFatalErrorLogging } from "../core/process-monitoring";
 import { config as dotenv } from "dotenv";
 dotenv();
 
@@ -16,6 +17,7 @@ import { startPerformanceMonitoring } from "../core/performance-monitoring";
 import { getAnonymizationPoolStats } from "../core/anonymization-pool";
 
 const logger = createLogger("streamer");
+installFatalErrorLogging("streamer");
 
 const app = express();
 app.use(monitorRequests("streamer"));