diff --git a/lib/github.msw.test.ts b/lib/github.msw.test.ts index 39923728e..2fd4c11f3 100644 --- a/lib/github.msw.test.ts +++ b/lib/github.msw.test.ts @@ -304,7 +304,7 @@ describe('MSW: error handling', () => { await expect(fetchUserProfile('octocat')).rejects.toThrow(); }); - it('handles network errors gracefully', async () => { + it('handles network errors gracefully by falling back to mock profile', async () => { globalThis.fetch = (() => { throw new TypeError('Failed to fetch'); }) as typeof fetch; @@ -312,6 +312,8 @@ describe('MSW: error handling', () => { process.env.GITHUB_PAT = 'test-token'; delete process.env.GITHUB_TOKEN; clearGitHubApiCacheForTests(); - await expect(fetchUserProfile('octocat')).rejects.toThrow(); + const result = await fetchUserProfile('octocat'); + expect(result.isOfflineFallback).toBe(true); + expect(result.login).toBe('octocat'); }); }); diff --git a/lib/github.test.ts b/lib/github.test.ts index d99b73b9b..278a083a5 100644 --- a/lib/github.test.ts +++ b/lib/github.test.ts @@ -283,10 +283,12 @@ describe('fetchGitHubContributions', () => { ); }); - it('throws when fetch itself rejects due to a network failure', async () => { + it('falls back to empty calendar when fetch itself rejects due to a network failure', async () => { vi.mocked(fetch).mockRejectedValue(new Error('Failed to fetch')); - await expect(fetchGitHubContributions('octocat')).rejects.toThrow('Failed to fetch'); + const result = await fetchGitHubContributions('octocat'); + expect(result.calendar.totalContributions).toBe(0); + expect(result.isOfflineFallback).toBe(true); }); it('throws the first GraphQL error when the API returns an errors array', async () => { @@ -352,15 +354,16 @@ describe('fetchGitHubContributions', () => { expect(fetch).toHaveBeenCalledTimes(2); }); - it('throws after exhausting all retries on repeated body-level RATE_LIMITED errors', async () => { + it('falls back to empty calendar after exhausting all retries on repeated body-level RATE_LIMITED errors', async () => { vi.mocked(fetch).mockResolvedValue( mockResponse({ errors: [{ type: 'RATE_LIMITED', message: 'API rate limit exceeded' }] }) ); const promise = fetchGitHubContributions('octocat'); - const assertion = expect(promise).rejects.toThrow('API Rate Limit Exceeded'); await vi.advanceTimersByTimeAsync(3500); - await assertion; + const result = await promise; + expect(result.calendar.totalContributions).toBe(0); + expect(result.isOfflineFallback).toBe(true); expect(fetch).toHaveBeenCalledTimes(4); }); }); @@ -790,15 +793,15 @@ describe('fetchContributedRepos', () => { await assertion; }); - it('throws on a rate-limited GraphQL 200 response instead of returning []', async () => { + it('falls back to [] on a rate-limited GraphQL 200 response', async () => { vi.mocked(fetch).mockResolvedValue( mockResponse({ errors: [{ type: 'RATE_LIMITED', message: 'API rate limit exceeded' }] }) ); const promise = fetchContributedRepos('octocat'); - const assertion = expect(promise).rejects.toThrow('API Rate Limit Exceeded'); await vi.advanceTimersByTimeAsync(3500); - await assertion; + const result = await promise; + expect(result).toEqual([]); }); it('does not cache the failure: a later call refetches and can succeed', async () => { diff --git a/lib/github.ts b/lib/github.ts index 67d3cefb6..814e09981 100644 --- a/lib/github.ts +++ b/lib/github.ts @@ -31,6 +31,48 @@ interface GitHubRepo { const MAX_RETRIES = 3; const BASE_DELAY_MS = 500; const MAX_RETRY_DELAY_MS = 5000; + +export function getJitteredBackoff(attempt: number): number { + const base = BASE_DELAY_MS * Math.pow(2, attempt); + const jitter = 0.5 + Math.random() * 0.5; + return Math.round(base * jitter); +} + +export function shouldFallbackOnError(err: unknown): boolean { + if (!err) return false; + const msg = err instanceof Error ? err.message : String(err); + + if ( + msg.includes('not found') || + msg.includes('Not Found') || + msg.includes('401') || + msg.includes('Unauthorized') || + msg.includes('token') || + msg.includes('PAT') || + msg.includes('Authorization') || + msg.includes('status 500') || + msg.includes('error: 500') || + msg.includes('Bad credentials') + ) { + return false; + } + + if ( + msg.includes('fetch') || + msg.includes('timeout') || + msg.includes('timed out') || + msg.includes('AbortError') || + msg.includes('rate limit') || + msg.includes('RATE_LIMITED') || + msg.includes('Rate Limit') || + err instanceof RateLimitError + ) { + return true; + } + + return false; +} + const GRAPHQL_TIMEOUT_MS = 8000; // 8s for GraphQL endpoint const REST_TIMEOUT_MS = 5000; // 5s for REST endpoints const ORG_MEMBER_LIMIT = 100; @@ -152,7 +194,7 @@ export async function fetchWithRetry( } throw fetchError; } - const delay = BASE_DELAY_MS * Math.pow(2, attempt); + const delay = getJitteredBackoff(attempt); await new Promise((resolve) => setTimeout(resolve, delay)); return fetchWithRetry(url, options, attempt + 1, timeoutMs, userToken); } @@ -196,7 +238,7 @@ export async function fetchWithRetry( } // Retry immediately with the next token if available if (attempt < MAX_RETRIES && tokens.length > 1) { - const delay = BASE_DELAY_MS * Math.pow(2, attempt); + const delay = getJitteredBackoff(attempt); await new Promise((resolve) => setTimeout(resolve, delay)); return fetchWithRetry(url, options, attempt + 1, timeoutMs, userToken); } @@ -227,7 +269,7 @@ export async function fetchWithRetry( if (attempt >= MAX_RETRIES) return res; - let delay = BASE_DELAY_MS * Math.pow(2, attempt); + let delay = getJitteredBackoff(attempt); if (retryAfter) { const parsed = parseInt(retryAfter, 10); if (!Number.isNaN(parsed) && String(parsed) === retryAfter) { @@ -257,7 +299,7 @@ export async function fetchWithRetry( const shouldRetry = res.status >= 500; if (!shouldRetry || attempt >= MAX_RETRIES) return res; - const delay = BASE_DELAY_MS * Math.pow(2, attempt); + const delay = getJitteredBackoff(attempt); await new Promise((resolve) => setTimeout(resolve, delay)); return fetchWithRetry(url, options, attempt + 1, timeoutMs, userToken); } @@ -337,9 +379,7 @@ async function fetchGraphQLWithRetry( if (!isBodyRateLimited) return res; - const delay = BASE_DELAY_MS * Math.pow(2, attempt); - if (delay > MAX_RETRY_DELAY_MS) return res; - + const delay = getJitteredBackoff(attempt); await new Promise((resolve) => setTimeout(resolve, delay)); return fetchGraphQLWithRetry(url, options, attempt + 1, timeoutMs, userToken); } @@ -467,6 +507,7 @@ interface GitHubUserProfile { location: string | null; type?: string; plan?: { name?: string } | null; + isOfflineFallback?: boolean; } function sanitizeUserProfile(profile: GitHubUserProfile): GitHubUserProfile { @@ -663,6 +704,21 @@ export function getCircuitTelemetry() { const FETCH_TIMEOUT_MS = 4000; const activeContributionsPromises = new Map>(); +export function getMockContributions(): ExtendedContributionData { + return { + calendar: { + totalContributions: 0, + weeks: [], + lastSyncedAt: new Date().toISOString(), + }, + repoContributions: [], + totalPRs: 0, + totalIssues: 0, + totalReviews: 0, + isOfflineFallback: true, + }; +} + export async function fetchGitHubContributions( username: string, options: FetchOptions = {} @@ -733,7 +789,26 @@ export async function fetchGitHubContributions( if (options.signal) { if (options.bypassCache || options.forceRefresh) { - return await loadWithTimeout(); + try { + return await loadWithTimeout(); + } catch (err: unknown) { + if (shouldFallbackOnError(err)) { + const staleData = await contributionsCache.get(key); + if (staleData) { + logger.warn('GitHub API fetch failed, falling back to stale cache', { + component: 'GitHub API', + username, + error: err, + }); + return { + ...staleData, + isOfflineFallback: true, + }; + } + return getMockContributions(username); + } + throw err; + } } const cached = await contributionsCache.get(key); if (cached !== null && !shouldFetch(cached)) { @@ -742,17 +817,20 @@ export async function fetchGitHubContributions( try { return await loadWithTimeout(); } catch (err: unknown) { - const staleData = await contributionsCache.get(key); - if (staleData) { - logger.warn('GitHub API fetch failed, falling back to stale cache', { - component: 'GitHub API', - username, - error: err, - }); - return { - ...staleData, - isOfflineFallback: true, - }; + if (shouldFallbackOnError(err)) { + const staleData = await contributionsCache.get(key); + if (staleData) { + logger.warn('GitHub API fetch failed, falling back to stale cache', { + component: 'GitHub API', + username, + error: err, + }); + return { + ...staleData, + isOfflineFallback: true, + }; + } + return getMockContributions(username); } throw err; } @@ -766,16 +844,19 @@ export async function fetchGitHubContributions( } return result; } catch (err: unknown) { - const staleData = await contributionsCache.get(key); - if (staleData) { - console.warn( - `[GitHub API] Fetch failed or timed out for "${username}", falling back to stale cache:`, - err - ); - return { - ...staleData, - isOfflineFallback: true, - }; + if (shouldFallbackOnError(err)) { + const staleData = await contributionsCache.get(key); + if (staleData) { + console.warn( + `[GitHub API] Fetch failed or timed out for "${username}", falling back to stale cache:`, + err + ); + return { + ...staleData, + isOfflineFallback: true, + }; + } + return getMockContributions(username); } throw err; } @@ -784,17 +865,20 @@ export async function fetchGitHubContributions( try { return await contributionsCache.getOrSet(key, coalescedLoad, LONG_CACHE_TTL, shouldFetch); } catch (err: unknown) { - const staleData = await contributionsCache.get(key); - if (staleData) { - logger.warn('GitHub API fetch failed, falling back to stale cache', { - component: 'GitHub API', - username, - error: err, - }); - return { - ...staleData, - isOfflineFallback: true, - }; + if (shouldFallbackOnError(err)) { + const staleData = await contributionsCache.get(key); + if (staleData) { + logger.warn('GitHub API fetch failed, falling back to stale cache', { + component: 'GitHub API', + username, + error: err, + }); + return { + ...staleData, + isOfflineFallback: true, + }; + } + return getMockContributions(username); } throw err; } @@ -958,6 +1042,23 @@ async function fetchContributionsUncached( }; } +export function getMockProfile(username: string): GitHubUserProfile { + return { + login: username, + name: username, + avatar_url: `https://github.com/${username}.png`, + public_repos: 0, + followers: 0, + following: 0, + created_at: new Date().toISOString(), + bio: 'Profile currently unavailable (offline fallback)', + location: null, + type: 'User', + plan: null, + isOfflineFallback: true, + }; +} + export async function fetchUserProfile( username: string, options: FetchOptions = {} @@ -969,8 +1070,41 @@ export async function fetchUserProfile( return fetchProfileUncached(encodedUsername, key, options); }; - if (options.bypassCache || options.forceRefresh) return load(); - return profileCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); + if (options.bypassCache || options.forceRefresh) { + try { + return await load(); + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await profileCache.get(key); + if (stale) { + logger.warn('GitHub API profile fetch failed, returning stale cache data', { + username, + error: err, + }); + return { ...stale, isOfflineFallback: true }; + } + return getMockProfile(username); + } + throw err; + } + } + + try { + return await profileCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await profileCache.get(key); + if (stale) { + logger.warn('GitHub API profile fetch failed, returning stale cache data', { + username, + error: err, + }); + return { ...stale, isOfflineFallback: true }; + } + return getMockProfile(username); + } + throw err; + } } async function fetchProfileUncached( @@ -1020,8 +1154,41 @@ export async function fetchUserRepos( return fetchReposUncached(encodedUsername, key, options); }; - if (options.bypassCache || options.forceRefresh) return load(); - return reposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); + if (options.bypassCache || options.forceRefresh) { + try { + return await load(); + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await reposCache.get(key); + if (stale) { + logger.warn('GitHub API repos fetch failed, returning stale cache data', { + username, + error: err, + }); + return stale; + } + return []; + } + throw err; + } + } + + try { + return await reposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await reposCache.get(key); + if (stale) { + logger.warn('GitHub API repos fetch failed, returning stale cache data', { + username, + error: err, + }); + return stale; + } + return []; + } + throw err; + } } async function fetchReposUncached( @@ -1439,13 +1606,45 @@ export async function fetchContributedRepos( return data?.data?.user?.repositoriesContributedTo?.nodes || []; }; - if (options.bypassCache) return load(); - if (options.forceRefresh) { - const fresh = await load(); - await contributedReposCache.set(key, fresh, GITHUB_CACHE_TTL_MS); - return fresh; + if (options.bypassCache || options.forceRefresh) { + try { + const fresh = await load(); + if (options.forceRefresh) { + await contributedReposCache.set(key, fresh, GITHUB_CACHE_TTL_MS); + } + return fresh; + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await contributedReposCache.get(key); + if (stale) { + logger.warn('GitHub API contributed repos fetch failed, returning stale cache data', { + username, + error: err, + }); + return stale; + } + return []; + } + throw err; + } + } + + try { + return await contributedReposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); + } catch (err) { + if (shouldFallbackOnError(err)) { + const stale = await contributedReposCache.get(key); + if (stale) { + logger.warn('GitHub API contributed repos fetch failed, returning stale cache data', { + username, + error: err, + }); + return stale; + } + return []; + } + throw err; } - return contributedReposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS); } export interface DeveloperScoreInput { diff --git a/src/utils/__tests__/graphqlSync.test.ts b/src/utils/__tests__/graphqlSync.test.ts index f58d1a992..8a7bdf789 100644 --- a/src/utils/__tests__/graphqlSync.test.ts +++ b/src/utils/__tests__/graphqlSync.test.ts @@ -48,9 +48,9 @@ describe('GraphQL Syncing Utility Integration Tests', () => { }), }); - // Verifies your utility safely surfaces network errors instead of an unhandled crash - await expect( - fetchGitHubContributions('attardekhushi78-cpu', { bypassCache: true }) - ).rejects.toThrow(); + // Verifies your utility gracefully falls back to empty calendar instead of an unhandled crash + const result = await fetchGitHubContributions('attardekhushi78-cpu', { bypassCache: true }); + expect(result.calendar.totalContributions).toBe(0); + expect(result.isOfflineFallback).toBe(true); }); }); diff --git a/tests/github-recovery.test.ts b/tests/github-recovery.test.ts new file mode 100644 index 000000000..0a5bf0ad3 --- /dev/null +++ b/tests/github-recovery.test.ts @@ -0,0 +1,149 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + fetchGitHubContributions, + fetchUserProfile, + fetchUserRepos, + getJitteredBackoff, + clearGitHubApiCacheForTests, +} from '../lib/github'; + +function mockResponse(body: unknown, status = 200, headers: Record = {}): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json', ...headers }, + }); +} + +describe('GitHub API Failure Recovery (Phase 3)', () => { + beforeEach(() => { + clearGitHubApiCacheForTests(); + vi.spyOn(global, 'fetch'); + process.env.GITHUB_PAT = 'test-token'; + }); + + afterEach(() => { + vi.restoreAllMocks(); + clearGitHubApiCacheForTests(); + }); + + describe('Jittered Backoff Calculations', () => { + it('verifies that getJitteredBackoff returns values within the correct bounds', () => { + // Attempt 0: base delay = 500ms. Bounds: [250, 500] + for (let i = 0; i < 100; i++) { + const delay = getJitteredBackoff(0); + expect(delay).toBeGreaterThanOrEqual(250); + expect(delay).toBeLessThanOrEqual(500); + } + + // Attempt 1: base delay = 1000ms. Bounds: [500, 1000] + for (let i = 0; i < 100; i++) { + const delay = getJitteredBackoff(1); + expect(delay).toBeGreaterThanOrEqual(500); + expect(delay).toBeLessThanOrEqual(1000); + } + }); + }); + + describe('fetchUserProfile Fallback', () => { + it('falls back to stale cache data on network failure', async () => { + // 1. Populate cache with a successful fetch first + vi.mocked(fetch).mockResolvedValueOnce( + mockResponse({ + login: 'octocat', + name: 'The Octocat', + avatar_url: 'https://github.com/octocat.png', + public_repos: 8, + followers: 20, + following: 9, + created_at: '2011-01-20T09:00:00Z', + }) + ); + const firstResult = await fetchUserProfile('octocat'); + expect(firstResult.login).toBe('octocat'); + + // 2. Mock a network failure for the second fetch + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + // 3. Force refresh or bypass cache to trigger error path, should get stale data with isOfflineFallback + const result = await fetchUserProfile('octocat', { forceRefresh: true }); + expect(result.login).toBe('octocat'); + expect(result.isOfflineFallback).toBe(true); + }); + + it('returns a mock placeholder profile if cache is empty on network failure', async () => { + // Mock network failure immediately when cache is empty + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + const result = await fetchUserProfile('some-new-user'); + expect(result.login).toBe('some-new-user'); + expect(result.isOfflineFallback).toBe(true); + expect(result.bio).toContain('offline fallback'); + }); + }); + + describe('fetchUserRepos Fallback', () => { + it('falls back to stale cache data on network failure', async () => { + // 1. Populate cache + vi.mocked(fetch).mockResolvedValueOnce( + mockResponse([{ name: 'repo-1', stargazers_count: 5, language: 'TypeScript' }]) + ); + const firstResult = await fetchUserRepos('octocat'); + expect(firstResult).toHaveLength(1); + + // 2. Mock network failure + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + // 3. Trigger error path + const result = await fetchUserRepos('octocat', { forceRefresh: true }); + expect(result).toHaveLength(1); + expect(result[0].name).toBe('repo-1'); + }); + + it('returns empty array if cache is empty on network failure', async () => { + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + const result = await fetchUserRepos('some-new-user'); + expect(result).toEqual([]); + }); + }); + + describe('fetchGitHubContributions Fallback', () => { + it('falls back to stale cache data on network failure', async () => { + // 1. Populate cache + const mockCalendar = { + totalContributions: 10, + weeks: [], + }; + vi.mocked(fetch).mockResolvedValueOnce( + mockResponse({ + data: { + user: { + contributionsCollection: { + contributionCalendar: mockCalendar, + commitContributionsByRepository: [], + }, + }, + }, + }) + ); + const firstResult = await fetchGitHubContributions('octocat'); + expect(firstResult.calendar.totalContributions).toBe(10); + + // 2. Mock network failure + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + // 3. Trigger error path + const result = await fetchGitHubContributions('octocat', { forceRefresh: true }); + expect(result.calendar.totalContributions).toBe(10); + expect(result.isOfflineFallback).toBe(true); + }); + + it('returns empty mock contributions calendar if cache is empty on network failure', async () => { + vi.mocked(fetch).mockRejectedValue(new Error('TypeError: Failed to fetch')); + + const result = await fetchGitHubContributions('some-new-user'); + expect(result.calendar.totalContributions).toBe(0); + expect(result.isOfflineFallback).toBe(true); + }); + }); +});