From 4ddb09edf15e781759134072c2b62a5358e96028 Mon Sep 17 00:00:00 2001
From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com>
Date: Tue, 21 Apr 2026 13:41:44 +0200
Subject: [PATCH] fix(server): racePeering normalizes handshake rejections and
 only warns on timeout win
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses code review on b22a7bd: (1) a rejected handshake now returns
{ status: 'failed', error } instead of throwing, keeping the structured
contract; (2) the "background handshake" warn only fires when the
timeout arm wins — not when the handshake is itself the race winner by
rejection. Timing tests migrated to vi.useFakeTimers for determinism.
Regression test added for the handshake-wins-by-rejection case.
---
 .../src/utils/federationPeering.test.ts       | 32 ++++++++++++++++---
 .../server/src/utils/federationPeering.ts     | 26 +++++++++------
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/packages/server/src/utils/federationPeering.test.ts b/packages/server/src/utils/federationPeering.test.ts
index 96f31624..3961dc0b 100644
--- a/packages/server/src/utils/federationPeering.test.ts
+++ b/packages/server/src/utils/federationPeering.test.ts
@@ -57,11 +57,15 @@ describe('racePeering', () => {
   });
 
   it('returns timeout when ensurePeered takes longer than the deadline', async () => {
+    vi.useFakeTimers();
     const stub = vi.fn((): Promise<EnsurePeeredResult> => new Promise(() => {
       // Never resolves — simulates a slow handshake.
     }));
-    const result = await racePeering('https://example.com', 50, stub);
+    const racePromise = racePeering('https://example.com', 50, stub);
+    await vi.advanceTimersByTimeAsync(50);
+    const result = await racePromise;
     expect(result).toEqual({ status: 'timeout' });
+    vi.useRealTimers();
   });
 
   it('returns rejected result verbatim when ensurePeered resolves with rejection', async () => {
@@ -73,20 +77,38 @@ describe('racePeering', () => {
     expect(result).toEqual({ status: 'rejected', error: 'peer denied' });
   });
 
-  it('attaches a warn-logged catch to the background promise so race-losing rejections do not emit unhandledRejection', async () => {
+  it('attaches a warn-logged catch to the background handshake when the timeout wins', async () => {
+    vi.useFakeTimers();
     const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
     const stub = vi.fn(() => new Promise<EnsurePeeredResult>((_, reject) => {
       setTimeout(() => reject(new Error('late failure')), 30);
     }));
-    const result = await racePeering('https://example.com', 10, stub);
+    const racePromise = racePeering('https://example.com', 10, stub);
+    await vi.advanceTimersByTimeAsync(10);
+    const result = await racePromise;
     expect(result).toEqual({ status: 'timeout' });
-    // Give the background promise time to reject.
-    await new Promise(r => setTimeout(r, 50));
+    await vi.advanceTimersByTimeAsync(30);
+    // Let microtasks flush so the .catch handler runs.
+    await Promise.resolve();
     expect(warnSpy).toHaveBeenCalledWith(
       expect.stringContaining('background handshake'),
       'https://example.com',
       expect.any(Error),
     );
+    vi.useRealTimers();
+    warnSpy.mockRestore();
+  });
+
+  it('normalizes a thrown handshake error into { status: failed } without emitting the background warn', async () => {
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const stub = vi.fn(async (): Promise<EnsurePeeredResult> => {
+      throw new Error('immediate handshake failure');
+    });
+    const result = await racePeering('https://example.com', 1_000, stub);
+    expect(result).toEqual({ status: 'failed', error: 'immediate handshake failure' });
+    // The handshake rejection was the race winner — no background warn should fire.
+    await Promise.resolve();
+    expect(warnSpy).not.toHaveBeenCalled();
     warnSpy.mockRestore();
   });
 });
diff --git a/packages/server/src/utils/federationPeering.ts b/packages/server/src/utils/federationPeering.ts
index 29fe7abc..2b582413 100644
--- a/packages/server/src/utils/federationPeering.ts
+++ b/packages/server/src/utils/federationPeering.ts
@@ -227,21 +227,29 @@ export async function racePeering(
 ): Promise<EnsurePeeredResult | { status: 'timeout' }> {
   const handshake = ensurePeeredFn(origin);
 
-  // Attach a warn-logged catch so a background arm that rejects AFTER the
-  // race loses does not trigger unhandledRejection. This runs regardless
-  // of which arm wins.
-  handshake.catch(err => {
-    console.warn('[federation] background handshake after call-relay race:', origin, err);
-  });
-
   let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
   const timeoutPromise = new Promise<{ status: 'timeout' }>(resolve => {
     timeoutHandle = setTimeout(() => resolve({ status: 'timeout' }), timeoutMs);
   });
 
+  let raceResult: EnsurePeeredResult | { status: 'timeout' };
   try {
-    return await Promise.race([handshake, timeoutPromise]);
-  } finally {
+    raceResult = await Promise.race([handshake, timeoutPromise]);
+  } catch (err) {
+    // ensurePeeredFn rejected as the race winner. Normalize to failed.
     if (timeoutHandle) clearTimeout(timeoutHandle);
+    const message = err instanceof Error ? err.message : 'Unknown handshake error';
+    return { status: 'failed', error: message };
   }
+  if (timeoutHandle) clearTimeout(timeoutHandle);
+
+  // Only when the timeout arm won is the background handshake still running.
+  // Guard its eventual rejection so we don't emit unhandledRejection.
+  if (raceResult.status === 'timeout') {
+    handshake.catch(err => {
+      console.warn('[federation] background handshake after call-relay race:', origin, err);
+    });
+  }
+
+  return raceResult;
 }