fix: avoid duplicate feedback fallback attempts

This commit is contained in:
Neil 2026-05-30 12:58:12 -07:00 committed by GitHub
parent 84e0af18c5
commit c5789ecc90
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 56 additions and 16 deletions

View File

@ -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, {

View File

@ -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)
}
}