From c5789ecc903fc9600809fbeb4186d0b1c3ff5cb3 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Sat, 30 May 2026 12:58:12 -0700
Subject: [PATCH] fix: avoid duplicate feedback fallback attempts

---
 src/main/ipc/feedback.test.ts | 27 +++++++++++++++++++++
 src/main/ipc/feedback.ts      | 45 ++++++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/src/main/ipc/feedback.test.ts b/src/main/ipc/feedback.test.ts
index 9753a1a8c..47543e0db 100644
--- a/src/main/ipc/feedback.test.ts
+++ b/src/main/ipc/feedback.test.ts
@@ -122,6 +122,33 @@ describe('submitFeedback', () => {
     expect(fetchMock).toHaveBeenCalledTimes(2)
   })
 
+  it('does not retry the fallback when the fallback fails after a primary server error', async () => {
+    vi.useFakeTimers()
+    fetchMock.mockImplementation((url: string, init?: RequestInit) => {
+      if (url.includes('api.onorca.dev')) {
+        return Promise.resolve({ ok: false, status: 500 } as Response)
+      }
+      return new Promise((_resolve, reject) => {
+        init?.signal?.addEventListener('abort', () => reject(new Error('fallback aborted')))
+      })
+    })
+
+    const result = submitFeedback({
+      feedback: 'primary 500 and fallback stalled',
+      submitAnonymously: false,
+      githubLogin: 'trusted-user',
+      githubEmail: 'trusted@example.com'
+    })
+    await vi.advanceTimersByTimeAsync(10_000)
+
+    await expect(Promise.race([result, Promise.resolve('pending')])).resolves.toEqual({
+      ok: false,
+      status: null,
+      error: 'fallback aborted'
+    })
+    expect(fetchMock).toHaveBeenCalledTimes(2)
+  })
+
   it('forces renderer IPC submissions onto the feedback lane', async () => {
     registerFeedbackHandlers()
     await handlers.get('feedback:submit')?.(null, {
diff --git a/src/main/ipc/feedback.ts b/src/main/ipc/feedback.ts
index b86ca2058..a207dd302 100644
--- a/src/main/ipc/feedback.ts
+++ b/src/main/ipc/feedback.ts
@@ -78,6 +78,33 @@ async function postFeedback(url: string, body: FeedbackSubmitBody): Promise<Resp
   }
 }
 
+function messageFromError(error: unknown): string {
+  return error instanceof Error ? error.message : String(error)
+}
+
+async function submitFallbackFeedback(
+  body: FeedbackSubmitBody,
+  primaryError?: unknown
+): Promise<FeedbackSubmitResult> {
+  try {
+    const fallback = await postFeedback(FEEDBACK_API_FALLBACK_URL, body)
+    if (fallback.ok) {
+      return { ok: true }
+    }
+    return { ok: false, status: fallback.status, error: `status ${fallback.status}` }
+  } catch (fallbackError) {
+    const message = messageFromError(fallbackError)
+    if (primaryError === undefined) {
+      return { ok: false, status: null, error: message }
+    }
+    return {
+      ok: false,
+      status: null,
+      error: `${messageFromError(primaryError)}; fallback: ${message}`
+    }
+  }
+}
+
 export async function submitFeedback(
   args: InternalFeedbackSubmitArgs
 ): Promise<FeedbackSubmitResult> {
@@ -91,28 +118,14 @@ export async function submitFeedback(
     // 404/5xx-style results and network errors — don't mask real 4xx responses
     // from a healthy host.
     if (res.status === 404 || res.status >= 500) {
-      const fallback = await postFeedback(FEEDBACK_API_FALLBACK_URL, body)
-      if (fallback.ok) {
-        return { ok: true }
-      }
-      return { ok: false, status: fallback.status, error: `status ${fallback.status}` }
+      return submitFallbackFeedback(body)
     }
     return { ok: false, status: res.status, error: `status ${res.status}` }
   } catch (error) {
     // Why: falling back on any network-level failure preserves the prior
     // behavior where DNS/connect failures on the primary host transparently
     // try the website-hosted versioned endpoint.
-    try {
-      const fallback = await postFeedback(FEEDBACK_API_FALLBACK_URL, body)
-      if (fallback.ok) {
-        return { ok: true }
-      }
-      return { ok: false, status: fallback.status, error: `status ${fallback.status}` }
-    } catch (fallbackError) {
-      const message = fallbackError instanceof Error ? fallbackError.message : String(fallbackError)
-      const primaryMessage = error instanceof Error ? error.message : String(error)
-      return { ok: false, status: null, error: `${primaryMessage}; fallback: ${message}` }
-    }
+    return submitFallbackFeedback(body, error)
   }
 }