From ac3ab72b522a3e57414ab87debccd2680acdaa2b Mon Sep 17 00:00:00 2001 From: TJ Baker <1617679+zaridan@users.noreply.github.com> Date: Mon, 22 Jun 2026 20:27:38 -0700 Subject: [PATCH] fix(jira): surface issue-search failures instead of a misleading empty list (#5958) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Orca Co-authored-by: brennanb2025 --- src/main/jira/issues.test.ts | 69 ++++++++++++++++- src/main/jira/issues.ts | 65 +++++++++++++--- src/renderer/src/components/TaskPage.tsx | 69 ++++++++++++++++- .../task-page-jira-load-state.test.ts | 66 ++++++++++++++++ .../components/task-page-jira-load-state.ts | 76 +++++++++++++++++++ src/renderer/src/i18n/locales/en.json | 3 +- src/renderer/src/i18n/locales/es.json | 3 +- src/renderer/src/i18n/locales/ja.json | 3 +- src/renderer/src/i18n/locales/ko.json | 3 +- src/renderer/src/i18n/locales/zh.json | 3 +- src/renderer/src/store/slices/jira.test.ts | 20 ++++- src/renderer/src/store/slices/jira.ts | 18 ++++- 12 files changed, 371 insertions(+), 27 deletions(-) create mode 100644 src/renderer/src/components/task-page-jira-load-state.test.ts create mode 100644 src/renderer/src/components/task-page-jira-load-state.ts diff --git a/src/main/jira/issues.test.ts b/src/main/jira/issues.test.ts index befbbeed9..49a49a460 100644 --- a/src/main/jira/issues.test.ts +++ b/src/main/jira/issues.test.ts @@ -18,10 +18,10 @@ vi.mock('./client', () => ({ jiraRequest: (...args: unknown[]) => jiraRequestMock(...args) })) -function makeEntry(): JiraClientForSite { +function makeEntry(id = 'site-1'): JiraClientForSite { return { site: { - id: 'site-1', + id, siteUrl: 'https://example.atlassian.net', email: 'ada@example.com', displayName: 'Example Jira', @@ -60,6 +60,71 @@ describe('Jira issue operations', () => { ).rejects.toThrow(error.message) }) + it('rejects single-site search failures so the UI can surface them', async () => { + getClientsMock.mockReturnValue([makeEntry('site-1')]) + jiraRequestMock.mockRejectedValueOnce(new Error('Forbidden')) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20, 'site-1')).rejects.toThrow('Forbidden') + }) + + it('includes Jira status codes in surfaced single-site search failures', async () => { + const error = Object.assign(new Error('Forbidden'), { status: 403 }) + getClientsMock.mockReturnValue([makeEntry('site-1')]) + jiraRequestMock.mockRejectedValueOnce(error) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20, 'site-1')).rejects.toThrow( + 'Error 403: Forbidden' + ) + }) + + it('keeps healthy sites when one site fails under an "all" search', async () => { + getClientsMock.mockReturnValue([makeEntry('site-1'), makeEntry('site-2')]) + jiraRequestMock.mockRejectedValueOnce(new Error('Forbidden')).mockResolvedValueOnce({ + issues: [{ id: '1', key: 'BRV-1', fields: { summary: 'Healthy' } }] + }) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20, 'all')).resolves.toMatchObject([ + { key: 'BRV-1', title: 'Healthy' } + ]) + }) + + it('keeps healthy sites when the saved selection fans out without an explicit site', async () => { + getClientsMock.mockReturnValue([makeEntry('site-1'), makeEntry('site-2')]) + jiraRequestMock.mockRejectedValueOnce(new Error('Forbidden')).mockResolvedValueOnce({ + issues: [{ id: '1', key: 'BRV-1', fields: { summary: 'Healthy' } }] + }) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20)).resolves.toMatchObject([ + { key: 'BRV-1', title: 'Healthy' } + ]) + }) + + it('surfaces an error when every site fails under an "all" search', async () => { + getClientsMock.mockReturnValue([makeEntry('site-1'), makeEntry('site-2')]) + jiraRequestMock + .mockRejectedValueOnce(new Error('Forbidden')) + .mockRejectedValueOnce(new Error('Service Unavailable')) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20, 'all')).rejects.toThrow('Forbidden') + }) + + it('prefers operational failures when every "all" search site fails', async () => { + const authError = new Error('Unauthorized') + const operationalError = new Error('Service Unavailable') + getClientsMock.mockReturnValue([makeEntry('site-1'), makeEntry('site-2')]) + isAuthErrorMock.mockImplementation((error) => error === authError) + jiraRequestMock.mockRejectedValueOnce(authError).mockRejectedValueOnce(operationalError) + const { searchIssues } = await import('./issues') + + await expect(searchIssues('project = ALP', 20, 'all')).rejects.toThrow('Service Unavailable') + expect(clearTokenMock).toHaveBeenCalledWith('site-1') + }) + it('paginates Jira project search results before sorting them', async () => { jiraRequestMock .mockResolvedValueOnce({ diff --git a/src/main/jira/issues.ts b/src/main/jira/issues.ts index 60a7dc49f..3bf823734 100644 --- a/src/main/jira/issues.ts +++ b/src/main/jira/issues.ts @@ -68,8 +68,38 @@ function clampLimit(limit: number | undefined, fallback = 30): number { return Math.min(Math.max(1, Number.isFinite(limit) ? Number(limit) : fallback), 100) } -function shouldThrowAuthError(selection: JiraSiteSelection | null | undefined): boolean { - return selection !== 'all' +type JiraIssueSearchFailure = { + error: unknown + auth: boolean +} + +function getErrorStatus(error: unknown): number | null { + if (!error || typeof error !== 'object' || !('status' in error)) { + return null + } + const status = (error as { status?: unknown }).status + return typeof status === 'number' && Number.isFinite(status) ? status : null +} + +function toIssueSearchFailureError(error: unknown): unknown { + const status = getErrorStatus(error) + if ( + status === null || + !(error instanceof Error) || + error.message.startsWith(`Error ${status}:`) + ) { + return error + } + return new Error(`Error ${status}: ${error.message}`) +} + +function shouldSurfaceSiteFailure( + selection: JiraSiteSelection | null | undefined, + entryCount: number +): boolean { + // getClients can resolve an omitted selection to the persisted 'all' choice; + // multi-entry reads need the same resilient fan-out policy as explicit 'all'. + return selection !== 'all' && entryCount <= 1 } function asRecord(value: unknown): JiraRecord { @@ -345,26 +375,37 @@ export async function searchIssues( return [] } const safeLimit = clampLimit(limit) + const failures: (JiraIssueSearchFailure | undefined)[] = Array.from({ length: entries.length }) + const surfaceSiteFailure = shouldSurfaceSiteFailure(siteId, entries.length) const results = await Promise.all( - entries.map(async (entry) => { + entries.map(async (entry, index) => { await acquire() try { return await searchIssuesForClient(entry, jql.trim(), safeLimit) } catch (error) { - if (isAuthError(error)) { + const authFailure = isAuthError(error) + if (authFailure) { clearToken(entry.site.id) - if (shouldThrowAuthError(siteId)) { - throw error - } - } else { - console.warn('[jira] searchIssues failed:', error) } - return [] + if (surfaceSiteFailure) { + throw toIssueSearchFailureError(error) + } + console.warn('[jira] searchIssues failed:', error) + failures[index] = { error: toIssueSearchFailureError(error), auth: authFailure } + return [] as JiraIssue[] } finally { release() } }) ) + // 'all' fan-out: only surface an error when every connected site failed, so a + // partial success (or a genuinely empty result) is not reported as an error. + const recordedFailures = failures.filter( + (failure): failure is JiraIssueSearchFailure => failure !== undefined + ) + if (recordedFailures.length === entries.length) { + throw (recordedFailures.find((failure) => !failure.auth) ?? recordedFailures[0]).error + } return entries.length === 1 ? results.flat().slice(0, safeLimit) : sortAndLimitIssues(results.flat(), safeLimit) @@ -388,7 +429,7 @@ export async function getIssue( } catch (error) { if (isAuthError(error)) { clearToken(entry.site.id) - if (shouldThrowAuthError(siteId)) { + if (shouldSurfaceSiteFailure(siteId, entries.length)) { throw error } } else { @@ -590,7 +631,7 @@ export async function listProjects(siteId?: JiraSiteSelection | null): Promise void +}): React.JSX.Element { + return ( + +
+ +
+
{error.title}
+ {error.details ? ( + <> + + + + +
+ {error.details} +
+ + + ) : null} +
+
+ + ) +} + function getLinearIssueGridTemplate(visibleProperties: ReadonlySet): string { const columns = ['96px', 'minmax(240px,1.55fr)'] if (visibleProperties.has('labels')) { @@ -4121,7 +4171,8 @@ export default function TaskPage(): React.JSX.Element { // Jira tab state const [jiraIssues, setJiraIssues] = useState([]) const [jiraLoading, setJiraLoading] = useState(false) - const [jiraError, setJiraError] = useState(null) + const [jiraError, setJiraError] = useState(null) + const [jiraErrorDetailsOpen, setJiraErrorDetailsOpen] = useState(false) const [jiraSearchInput, setJiraSearchInput] = useState('') const [appliedJiraSearch, setAppliedJiraSearch] = useState('') const [activeJiraPreset, setActiveJiraPreset] = useState('assigned') @@ -7231,6 +7282,7 @@ export default function TaskPage(): React.JSX.Element { let cancelled = false setJiraLoading(true) setJiraError(null) + setJiraErrorDetailsOpen(false) const trimmed = appliedJiraSearch.trim() const request = @@ -7252,7 +7304,9 @@ export default function TaskPage(): React.JSX.Element { if (cancelled) { return } - setJiraError(err instanceof Error ? err.message : 'Failed to load Jira issues.') + const failureState = createTaskPageJiraLoadFailureState(err) + setJiraIssues(failureState.issues) + setJiraError(failureState.error) setJiraLoading(false) }) @@ -9387,11 +9441,18 @@ export default function TaskPage(): React.JSX.Element { className="min-h-0 flex-1 overflow-y-auto scrollbar-sleek" style={{ scrollbarGutter: 'stable' }} > - {(jiraStatus.credentialError ?? jiraError) ? ( + {jiraStatus.credentialError ? (
- {jiraStatus.credentialError ?? jiraError} + {jiraStatus.credentialError}
) : null} + {!jiraStatus.credentialError && jiraError ? ( + + ) : null} {jiraLoading && jiraIssues.length === 0 ? (
diff --git a/src/renderer/src/components/task-page-jira-load-state.test.ts b/src/renderer/src/components/task-page-jira-load-state.test.ts new file mode 100644 index 000000000..a5a151386 --- /dev/null +++ b/src/renderer/src/components/task-page-jira-load-state.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import { createTaskPageJiraLoadFailureState } from './task-page-jira-load-state' + +describe('TaskPage Jira load state', () => { + it('explains Jira forbidden errors while clearing stale issues', () => { + expect(createTaskPageJiraLoadFailureState(new Error('Forbidden'))).toEqual({ + issues: [], + error: { + title: + 'Error 403: Jira denied access to this issue search. Check project permissions or try a different JQL query.', + details: 'Forbidden' + } + }) + }) + + it('keeps raw provider detail separate from the Jira status summary', () => { + expect(createTaskPageJiraLoadFailureState(new Error('Error 403: XSRF check failed'))).toEqual({ + issues: [], + error: { + title: + 'Error 403: Jira denied access to this issue search. Check project permissions or try a different JQL query.', + details: 'XSRF check failed' + } + }) + }) + + it('explains malformed JQL errors', () => { + expect(createTaskPageJiraLoadFailureState(new Error('Malformed JQL'))).toEqual({ + issues: [], + error: { + title: "Jira couldn't run this JQL query. Check the syntax and try again.", + details: 'Malformed JQL' + } + }) + }) + + it('explains network errors', () => { + expect(createTaskPageJiraLoadFailureState(new Error('Network request failed'))).toEqual({ + issues: [], + error: { + title: "Couldn't reach Jira. Check your connection and try again.", + details: 'Network request failed' + } + }) + }) + + it('explains Jira server errors', () => { + expect(createTaskPageJiraLoadFailureState(new Error('Service Unavailable'))).toEqual({ + issues: [], + error: { + title: 'Error 503: Jira had a server error while loading issues. Try again in a moment.', + details: 'Service Unavailable' + } + }) + }) + + it('uses the generic load error for non-Error rejections', () => { + expect(createTaskPageJiraLoadFailureState('failed')).toEqual({ + issues: [], + error: { + title: "Couldn't load Jira issues. Try again in a moment.", + details: 'Failed to load Jira issues.' + } + }) + }) +}) diff --git a/src/renderer/src/components/task-page-jira-load-state.ts b/src/renderer/src/components/task-page-jira-load-state.ts new file mode 100644 index 000000000..77826b781 --- /dev/null +++ b/src/renderer/src/components/task-page-jira-load-state.ts @@ -0,0 +1,76 @@ +import type { JiraIssue } from '../../../shared/types' + +export type TaskPageJiraLoadError = { + title: string + details: string | null +} + +export type TaskPageJiraLoadFailureState = { + issues: JiraIssue[] + error: TaskPageJiraLoadError +} + +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : 'Failed to load Jira issues.' +} + +function getErrorCode(message: string): number | null { + const explicit = /^Error\s+(\d{3})\b/i.exec(message)?.[1] + if (explicit) { + return Number(explicit) + } + if (/\bforbidden\b/i.test(message)) { + return 403 + } + if (/\bunauthorized\b|\bunauthenticated\b/i.test(message)) { + return 401 + } + if (/\btoo many requests\b|\brate limit\b/i.test(message)) { + return 429 + } + if (/\bservice unavailable\b/i.test(message)) { + return 503 + } + return null +} + +function getErrorDetails(message: string, code: number | null): string | null { + const normalized = + code === null ? message : message.replace(new RegExp(`^Error\\s+${code}:\\s*`, 'i'), '') + return normalized.trim() || null +} + +function getIssueSearchErrorSummary(message: string, code: number | null): string { + if (code === 401) { + return 'Jira authentication failed. Reconnect Jira in Settings, then try again.' + } + if (code === 403) { + return 'Jira denied access to this issue search. Check project permissions or try a different JQL query.' + } + if (code === 429) { + return 'Jira rate-limited this issue search. Try again in a moment.' + } + if (code !== null && code >= 500) { + return 'Jira had a server error while loading issues. Try again in a moment.' + } + if (/\bjql\b|\bsyntax\b/i.test(message)) { + return "Jira couldn't run this JQL query. Check the syntax and try again." + } + if (/\bnetwork\b|\bfetch failed\b|\btimed? ?out\b|\beconn/i.test(message)) { + return "Couldn't reach Jira. Check your connection and try again." + } + return "Couldn't load Jira issues. Try again in a moment." +} + +export function createTaskPageJiraLoadFailureState(error: unknown): TaskPageJiraLoadFailureState { + const message = getErrorMessage(error) + const code = getErrorCode(message) + const summary = getIssueSearchErrorSummary(message, code) + return { + issues: [], + error: { + title: code === null ? summary : `Error ${code}: ${summary}`, + details: getErrorDetails(message, code) + } + } +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 20488fa7e..4483a71a2 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1601,7 +1601,8 @@ "ff90d0abc7": "Start workspace from {{value0}}", "fe28c9821f": "view", "8d1e17a3ef": "Open {{value0}} in GitHub", - "4ac8ff2275": "Open {{value0}} in Jira" + "4ac8ff2275": "Open {{value0}} in Jira", + "40eaf2c27c": "Details" }, "Terminal": { "73768427cf": "Close", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 23d644fa9..a60a6d2db 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -1601,7 +1601,8 @@ "ff90d0abc7": "Iniciar espacio de trabajo desde {{value0}}", "fe28c9821f": "vista", "8d1e17a3ef": "Abra {{value0}} en GitHub", - "4ac8ff2275": "Abrir {{value0}} en Jira" + "4ac8ff2275": "Abrir {{value0}} en Jira", + "40eaf2c27c": "Details" }, "Terminal": { "73768427cf": "Cerca", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index dcce2ac18..b25277abe 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -1601,7 +1601,8 @@ "ff90d0abc7": "{{value0}} からワークスペースを開始", "fe28c9821f": "view", "8d1e17a3ef": "{{value0}} を GitHub で開く", - "4ac8ff2275": "{{value0}} を Jira で開く" + "4ac8ff2275": "{{value0}} を Jira で開く", + "40eaf2c27c": "Details" }, "Terminal": { "73768427cf": "閉じる", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 31d7c09eb..beda6a43a 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -1601,7 +1601,8 @@ "ff90d0abc7": "{{value0}}에서 워크스페이스 시작", "fe28c9821f": "보기", "8d1e17a3ef": "GitHub에서 {{value0}} 열기", - "4ac8ff2275": "Jira에서 {{value0}} 열기" + "4ac8ff2275": "Jira에서 {{value0}} 열기", + "40eaf2c27c": "Details" }, "Terminal": { "73768427cf": "닫기", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index a2a27399a..0723b0e1e 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -1601,7 +1601,8 @@ "ff90d0abc7": "从 {{value0}} 开始工作区", "fe28c9821f": "视图", "8d1e17a3ef": "在 GitHub 中打开 {{value0}}", - "4ac8ff2275": "在 Jira 中打开 {{value0}}" + "4ac8ff2275": "在 Jira 中打开 {{value0}}", + "40eaf2c27c": "Details" }, "Terminal": { "73768427cf": "关闭", diff --git a/src/renderer/src/store/slices/jira.test.ts b/src/renderer/src/store/slices/jira.test.ts index ebe427f44..f1dabcb24 100644 --- a/src/renderer/src/store/slices/jira.test.ts +++ b/src/renderer/src/store/slices/jira.test.ts @@ -391,14 +391,30 @@ describe('createJiraSlice credential errors', () => { }) }) - it('keeps Jira connected when an issue read hits endpoint-level forbidden access', async () => { + it('surfaces endpoint-level forbidden errors without disconnecting Jira', async () => { const store = createTestStore() store.setState({ jiraStatus: { connected: true, viewer: null, selectedSiteId: 'site-1' } }) jiraListIssues.mockRejectedValueOnce(new Error('Forbidden')) - await expect(store.getState().listJiraIssues('assigned', 30)).resolves.toEqual([]) + // A non-auth failure must reject so the Tasks panel can show a real error + // instead of a misleading empty list, while keeping the session connected. + await expect(store.getState().listJiraIssues('assigned', 30)).rejects.toThrow('Forbidden') + + expect(store.getState().jiraStatus.connected).toBe(true) + }) + + it('surfaces endpoint-level search errors without disconnecting Jira', async () => { + const store = createTestStore() + store.setState({ + jiraStatus: { connected: true, viewer: null, selectedSiteId: 'site-1' } + }) + jiraSearchIssues.mockRejectedValueOnce(new Error('Malformed JQL')) + + await expect(store.getState().searchJiraIssues('project =', 30)).rejects.toThrow( + 'Malformed JQL' + ) expect(store.getState().jiraStatus.connected).toBe(true) }) diff --git a/src/renderer/src/store/slices/jira.ts b/src/renderer/src/store/slices/jira.ts index 1498f312e..fd084a3d9 100644 --- a/src/renderer/src/store/slices/jira.ts +++ b/src/renderer/src/store/slices/jira.ts @@ -496,7 +496,14 @@ export const createJiraSlice: StateCreator = (set, ) { set({ jiraStatus: { connected: false, viewer: null } }) } - return [] + // Credential/auth failures are surfaced through connection state, so they + // keep the empty-list contract. Other failures (forbidden, bad JQL, + // network, 5xx) reject so the Tasks panel can show a real error instead + // of a misleading "No issues found". + if (isIntegrationCredentialDecryptionError(error) || looksLikeAuthError(error)) { + return [] + } + throw error }) .finally(() => { if (inflightSearchRequests.get(cacheKey) === entry) { @@ -583,7 +590,14 @@ export const createJiraSlice: StateCreator = (set, ) { set({ jiraStatus: { connected: false, viewer: null } }) } - return [] + // Credential/auth failures are surfaced through connection state, so they + // keep the empty-list contract. Other failures (forbidden, bad JQL, + // network, 5xx) reject so the Tasks panel can show a real error instead + // of a misleading "No issues found". + if (isIntegrationCredentialDecryptionError(error) || looksLikeAuthError(error)) { + return [] + } + throw error }) .finally(() => { if (inflightListRequests.get(cacheKey) === entry) {