fix(gateway): isolate Gravatar waiter deadlines

src/gateway/user-profiles-http.test.ts

@@ -1,6 +1,8 @@
 import { createHash } from "node:crypto";
+import { EventEmitter } from "node:events";
 import type { IncomingMessage, ServerResponse } from "node:http";
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createDeferred } from "../../test/helpers/promise.js";
 import { handleUserProfileAvatarHttpRequest } from "./user-profiles-http.js";
 
 const authorizeScopedUserProfileAvatarHttpRequestOrReply = vi.hoisted(() => vi.fn());
@@ -35,14 +37,56 @@ function response() {
   const writeHead = vi.fn();
   return {
     end,
-    response: { end, setHeader, writeHead } as unknown as ServerResponse,
+    response: Object.assign(new EventEmitter(), {
+      destroyed: false,
+      end,
+      setHeader,
+      socket: null,
+      writeHead,
+    }) as unknown as ServerResponse,
     setHeader,
     writeHead,
   };
 }
 
 function request(path: string, headers: Record<string, string> = {}) {
-  return { method: "GET", url: path, headers } as unknown as IncomingMessage;
+  return {
+    method: "GET",
+    url: path,
+    headers,
+    socket: Object.assign(new EventEmitter(), { destroyed: false }),
+  } as unknown as IncomingMessage;
+}
+
+function disconnectableRequest(path: string) {
+  const socket = Object.assign(new EventEmitter(), { destroyed: false });
+  return {
+    request: { method: "GET", url: path, headers: {}, socket } as unknown as IncomingMessage,
+    socket,
+  };
+}
+
+function waitForDeferredResponse(
+  deferred: { promise: Promise<Response> },
+  signal: AbortSignal | null | undefined,
+): Promise<Response> {
+  return new Promise<Response>((resolve, reject) => {
+    const onAbort = () => {
+      const reason = signal?.reason;
+      reject(reason instanceof Error ? reason : new Error(String(reason)));
+    };
+    signal?.addEventListener("abort", onAbort, { once: true });
+    void deferred.promise.then(
+      (value) => {
+        signal?.removeEventListener("abort", onAbort);
+        resolve(value);
+      },
+      (error: unknown) => {
+        signal?.removeEventListener("abort", onAbort);
+        reject(error instanceof Error ? error : new Error(String(error)));
+      },
+    );
+  });
 }
 
 describe("profile avatar HTTP endpoint", () => {
@@ -397,6 +441,128 @@ describe("profile avatar HTTP endpoint", () => {
     expect(res.end).toHaveBeenCalledWith(new Uint8Array([2, 2, 2]));
   });
 
+  it("keeps a shared Gravatar fetch alive after its first waiter's deadline", async () => {
+    const firstProfileId = "profile-shared-deadline-first";
+    const secondProfileId = "profile-shared-deadline-second";
+    const primaryHash = emailHash("shared-deadline-primary@example.com");
+    const sharedHash = emailHash("shared-deadline@example.com");
+    const sharedResponse = createDeferred<Response>();
+    const totalDeadlines: AbortController[] = [];
+    const timeoutSpy = vi.spyOn(AbortSignal, "timeout").mockImplementation((delay) => {
+      const controller = new AbortController();
+      if (delay === 6_000) {
+        totalDeadlines.push(controller);
+      }
+      return controller.signal;
+    });
+    getProfileAvatar.mockReturnValue(undefined);
+    getUserProfileListItem.mockImplementation((profileId: string) => ({
+      id: profileId,
+      emails:
+        profileId === firstProfileId
+          ? ["shared-deadline-primary@example.com", "shared-deadline@example.com"]
+          : ["shared-deadline@example.com"],
+      hasAvatar: false,
+    }));
+    const fetchImpl = vi.fn((input: URL | RequestInfo, init?: RequestInit) => {
+      if (fetchUrl(input).includes(primaryHash)) {
+        return Promise.resolve(new Response(null, { status: 404 }));
+      }
+      expect(fetchUrl(input)).toContain(sharedHash);
+      return waitForDeferredResponse(sharedResponse, init?.signal);
+    });
+    const first = response();
+    const second = response();
+
+    try {
+      const firstRequest = handleUserProfileAvatarHttpRequest(
+        request("/ignored-by-handler"),
+        first.response,
+        `/api/users/${firstProfileId}/avatar`,
+        { auth: {} as never, fetchImpl },
+      );
+      await vi.waitFor(() => expect(fetchImpl).toHaveBeenCalledTimes(2));
+      const secondRequest = handleUserProfileAvatarHttpRequest(
+        request("/ignored-by-handler"),
+        second.response,
+        `/api/users/${secondProfileId}/avatar`,
+        { auth: {} as never, fetchImpl },
+      );
+      await vi.waitFor(() => expect(totalDeadlines).toHaveLength(2));
+
+      totalDeadlines[0]?.abort();
+      await firstRequest;
+      sharedResponse.resolve(
+        new Response(new Uint8Array([7, 8, 9]), {
+          status: 200,
+          headers: { "content-type": "image/png" },
+        }),
+      );
+      await secondRequest;
+
+      expect(first.response.statusCode).toBe(502);
+      expect(second.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
+      expect(second.end).toHaveBeenCalledWith(new Uint8Array([7, 8, 9]));
+      expect(fetchImpl).toHaveBeenCalledTimes(2);
+    } finally {
+      timeoutSpy.mockRestore();
+    }
+  });
+
+  it("retires a disconnected waiter without aborting its shared Gravatar fetch", async () => {
+    const profileId = "profile-shared-disconnect";
+    const sharedResponse = createDeferred<Response>();
+    getProfileAvatar.mockReturnValue(undefined);
+    getUserProfileListItem.mockReturnValue({
+      id: profileId,
+      emails: ["shared-disconnect@example.com"],
+      hasAvatar: false,
+    });
+    const fetchImpl = vi.fn((_input: URL | RequestInfo, init?: RequestInit) =>
+      waitForDeferredResponse(sharedResponse, init?.signal),
+    );
+    const firstReq = disconnectableRequest("/ignored-by-handler");
+    const firstRes = response();
+    const secondRes = response();
+    let firstSettled = false;
+
+    const firstRequest = handleUserProfileAvatarHttpRequest(
+      firstReq.request,
+      firstRes.response,
+      `/api/users/${profileId}/avatar`,
+      { auth: {} as never, fetchImpl },
+    ).then((handled) => {
+      firstSettled = true;
+      return handled;
+    });
+    await vi.waitFor(() => expect(fetchImpl).toHaveBeenCalledOnce());
+    const secondRequest = handleUserProfileAvatarHttpRequest(
+      request("/ignored-by-handler"),
+      secondRes.response,
+      `/api/users/${profileId}/avatar`,
+      { auth: {} as never, fetchImpl },
+    );
+
+    firstReq.socket.emit("close");
+    await vi.waitFor(() => expect(firstSettled).toBe(true));
+    expect(firstRes.writeHead).not.toHaveBeenCalled();
+    expect(firstRes.end).not.toHaveBeenCalled();
+    expect(firstReq.socket.listenerCount("close")).toBe(0);
+
+    sharedResponse.resolve(
+      new Response(new Uint8Array([3, 2, 1]), {
+        status: 200,
+        headers: { "content-type": "image/png" },
+      }),
+    );
+    await expect(firstRequest).resolves.toBe(true);
+    await secondRequest;
+
+    expect(secondRes.writeHead).toHaveBeenCalledWith(200, expect.any(Object));
+    expect(secondRes.end).toHaveBeenCalledWith(new Uint8Array([3, 2, 1]));
+    expect(fetchImpl).toHaveBeenCalledOnce();
+  });
+
   it("caps the Gravatar fan-out so a profile with many linked emails is bounded", async () => {
     const profileId = "profile-many-emails";
     const emails = Array.from({ length: 12 }, (_, index) => `many-${index}@example.com`);

src/gateway/user-profiles-http.ts

@@ -13,7 +13,7 @@ import {
 import type { AuthRateLimiter } from "./auth-rate-limit.js";
 import type { ResolvedGatewayAuth } from "./auth.js";
 import { parseControlUiUserAvatarPath } from "./control-ui-contract.js";
-import { sendJson, sendMethodNotAllowed } from "./http-common.js";
+import { sendJson, sendMethodNotAllowed, watchClientDisconnect } from "./http-common.js";
 import { matchesHttpIfNoneMatch } from "./http-conditional.js";
 import {
   authorizeScopedUserProfileAvatarHttpRequestOrReply,
@@ -201,13 +201,11 @@ async function cancelGravatarBody(body: ReadableStream<Uint8Array> | null): Prom
 async function fetchGravatar(
   hash: string,
   fetchImpl: typeof globalThis.fetch,
-  deadline?: AbortSignal,
 ): Promise<GravatarResult> {
   try {
-    const perCall = AbortSignal.timeout(GRAVATAR_FETCH_TIMEOUT_MS);
     const response = await fetchImpl(`${GRAVATAR_BASE_URL}/${hash}?s=256&d=404`, {
       headers: { Accept: "image/webp,image/png,image/jpeg,image/gif" },
-      signal: deadline ? AbortSignal.any([deadline, perCall]) : perCall,
+      signal: AbortSignal.timeout(GRAVATAR_FETCH_TIMEOUT_MS),
     });
     if (response.status === 404) {
       await cancelGravatarBody(response.body);
@@ -239,7 +237,7 @@ async function fetchGravatar(
 
 async function resolveGravatar(
   hash: string,
-  options: { fetchImpl: typeof globalThis.fetch; nowMs: () => number; deadline?: AbortSignal },
+  options: { fetchImpl: typeof globalThis.fetch; nowMs: () => number },
 ): Promise<GravatarResult> {
   const cached = getCachedGravatar(hash, options.nowMs());
   if (cached) {
@@ -249,7 +247,7 @@ async function resolveGravatar(
     gravatarRequests,
     hash,
     async () => {
-      const result = await fetchGravatar(hash, options.fetchImpl, options.deadline);
+      const result = await fetchGravatar(hash, options.fetchImpl);
       if (result.kind !== "error") {
         cacheGravatar(hash, result, options.nowMs());
       }
@@ -259,6 +257,35 @@ async function resolveGravatar(
   );
 }
 
+async function waitForGravatar(
+  result: Promise<GravatarResult>,
+  signal: AbortSignal,
+): Promise<GravatarResult | undefined> {
+  if (signal.aborted) {
+    return undefined;
+  }
+  return await new Promise<GravatarResult | undefined>((resolve, reject) => {
+    const onAbort = () => {
+      signal.removeEventListener("abort", onAbort);
+      resolve(undefined);
+    };
+    signal.addEventListener("abort", onAbort, { once: true });
+    void result.then(
+      (value) => {
+        signal.removeEventListener("abort", onAbort);
+        resolve(value);
+      },
+      (error: unknown) => {
+        signal.removeEventListener("abort", onAbort);
+        reject(error instanceof Error ? error : new Error(String(error)));
+      },
+    );
+    if (signal.aborted) {
+      onAbort();
+    }
+  });
+}
+
 function sendAvatar(
   req: IncomingMessage,
   res: ServerResponse,
@@ -382,29 +409,53 @@ export async function handleUserProfileAvatarHttpRequest(
 
   // Resolve linked emails sequentially and stop at the first hit: the primary
   // email keeps precedence, and a secondary email's hash is disclosed to
-  // Gravatar only once the earlier one is a definite miss. A single shared
-  // deadline bounds the total wait, so an unreachable Gravatar cannot stall the
-  // held connection by one timeout per linked email.
+  // Gravatar only once the earlier one is a definite miss. Shared hash fetches
+  // own their intrinsic timeout; this waiter independently owns the total HTTP
+  // deadline and disconnect lifecycle.
+  const clientAbort = new AbortController();
+  const stopWatchingDisconnect = watchClientDisconnect(req, res, clientAbort);
   const deadline = AbortSignal.timeout(GRAVATAR_TOTAL_TIMEOUT_MS);
-  let transientFailure = false;
-  for (const hash of hashes) {
-    const result = await resolveGravatar(hash, {
-      fetchImpl: opts.fetchImpl ?? globalThis.fetch,
-      nowMs: opts.nowMs ?? Date.now,
-      deadline,
-    });
-    if (result.kind === "hit") {
-      sendAvatar(req, res, result, "private, max-age=0, must-revalidate");
+  const waiterSignal = AbortSignal.any([clientAbort.signal, deadline]);
+  try {
+    let transientFailure = false;
+    for (const hash of hashes) {
+      if (clientAbort.signal.aborted) {
+        return true;
+      }
+      if (deadline.aborted) {
+        transientFailure = true;
+        break;
+      }
+      const result = await waitForGravatar(
+        resolveGravatar(hash, {
+          fetchImpl: opts.fetchImpl ?? globalThis.fetch,
+          nowMs: opts.nowMs ?? Date.now,
+        }),
+        waiterSignal,
+      );
+      if (!result || waiterSignal.aborted) {
+        if (clientAbort.signal.aborted) {
+          return true;
+        }
+        transientFailure = true;
+        break;
+      }
+      if (result.kind === "hit") {
+        sendAvatar(req, res, result, "private, max-age=0, must-revalidate");
+        return true;
+      }
+      transientFailure ||= result.kind === "error";
+    }
+    if (clientAbort.signal.aborted) {
       return true;
     }
-    transientFailure ||= result.kind === "error";
-    if (deadline.aborted) {
-      break;
-    }
+    transientFailure ||= deadline.aborted;
+    sendJson(res, transientFailure ? 502 : 404, {
+      ok: false,
+      error: { type: transientFailure ? "avatar_upstream_unavailable" : "not_found" },
+    });
+    return true;
+  } finally {
+    stopWatchingDisconnect();
   }
-  sendJson(res, transientFailure ? 502 : 404, {
-    ok: false,
-    error: { type: transientFailure ? "avatar_upstream_unavailable" : "not_found" },
-  });
-  return true;
 }