fix(mobile): await each per-repo GitHub count so one failure is a zero

Returning the promise from inside the try let a rejected count escape the
per-repo catch, reject the whole batch and reach the caller, which fires this
without a handler. The inline call it replaced awaited and reported zero.

The new test drives the real batching shape and fails if the await is removed.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
2026-09-09 01:45:44 -04:00
parent 1b86a6842b
commit aefdf2d034
3 changed files with 101 additions and 3 deletions
@@ -25,9 +25,9 @@ const hash = (parts: string[] | string): string =>
* plus the row-target locals the adapters take in place of inline slug/number checks. Diff hooks,
* declarations and styles are untouched.
*/
const SCREEN_HOOKS = '0ef59ab331ad320a5ccabe8c227616af263ad2c468a805a6743dde8c186d7027'
const SCREEN_HOOKS = '9b2ac231e63169d3896ce30a3053262d30bc8f085456d75cbade522420205b02'
const DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f'
const STATEMENTS = 'e5d84617b7acbbdcaf17129d19670fac62a0399d967d9a6fc225cb9b807ae886'
const STATEMENTS = 'dd86f1ff89bf0bdbc3fbf159349d972a50d2dfa2607484848a34c4312b36ae60'
const DECLARATIONS = 'cff54172af17a877789be1479c2eb6ca97d83c3e31dd831cd59395962f2b4c4a'
const SEMANTICS = 'f767906884b93537f2c6369d6d0bd2d4cb39b4314c31cca9d8f9e5e9b78a75ee'
const STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a'
@@ -0,0 +1,95 @@
import { createElement } from 'react'
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
import { afterEach, describe, expect, it, vi } from 'vitest'
// The module under test reaches the Tasks barrel, which pulls React Native in. Both barrels are
// stubbed with self-contained equivalents; `mapWithConcurrency` reproduces the shipped one
// (`mobile-tasks-item-mapping.ts`), which awaits each worker and so rejects the whole batch if
// any worker rejects. That is precisely the propagation this test pins the guard against.
vi.mock('./mobile-tasks-dependencies', () => ({
CROSS_REPO_DISPLAY_LIMIT: 200,
PER_REPO_FETCH_LIMIT: 100,
extractGitHubIssueSourceError: () => null,
extractGitHubIssueSourceFallback: () => null,
isGitHubWorkItemsSshRemoteRequiredError: () => false,
useCallback: <T,>(callback: T): T => callback
}))
vi.mock('./mobile-tasks-legacy-foundation', () => ({
GITHUB_REPO_CONCURRENCY: 3,
createGitHubTask: (item: unknown) => item,
async mapWithConcurrency<T, R>(
items: T[],
limit: number,
worker: (item: T) => Promise<R>
): Promise<R[]> {
const results: R[] = []
let nextIndex = 0
async function run(): Promise<void> {
while (nextIndex < items.length) {
const index = nextIndex
nextIndex += 1
results[index] = await worker(items[index]!)
}
}
await Promise.all(Array.from({ length: Math.min(limit, items.length) }, () => run()))
return results
},
reconcileTeamSelection: () => new Set<string>(),
scopeGitHubTaskSearch: (query: string): string => query,
taskTime: () => 0
}))
const { useMobileTasksProviderLoadActions } =
await import('./use-mobile-tasks-provider-load-actions')
type CountOperations = { countGitHub: (payload: { repoId: string }) => Promise<number> }
type LoadActions = { countGitHubItems: (operations: unknown, repos: unknown[]) => Promise<number> }
const repo = (id: string): { id: string } => ({ id })
describe('github work-item counting', () => {
let renderer: ReactTestRenderer | null = null
function mount(): LoadActions {
let model: LoadActions | null = null
function Probe(): null {
model = useMobileTasksProviderLoadActions({
appliedQuery: 'is:open',
githubKind: 'issues'
} as never) as unknown as LoadActions
return null
}
act(() => {
renderer = create(createElement(Probe))
})
if (!model) {
throw new Error('hook did not render')
}
return model
}
afterEach(() => {
act(() => renderer?.unmount())
renderer = null
vi.restoreAllMocks()
})
it('reads a failed per-repo count as zero and still resolves the total', async () => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
const countGitHub = vi.fn(async ({ repoId }: { repoId: string }) => {
if (repoId === 'repo-broken') {
throw new Error('count failed')
}
return repoId === 'repo-a' ? 3 : 4
})
const operations: CountOperations = { countGitHub }
const model = mount()
// Returning the promise instead of awaiting it would let this rejection escape the per-repo
// catch and reject the whole batch, and the only caller fires this with no handler.
await expect(
model.countGitHubItems(operations, [repo('repo-a'), repo('repo-broken'), repo('repo-b')])
).resolves.toBe(7)
expect(countGitHub).toHaveBeenCalledTimes(3)
})
})
@@ -167,7 +167,10 @@ export function useMobileTasksProviderLoadActions(model: RuntimeHydrationModel)
GITHUB_REPO_CONCURRENCY,
async (repo) => {
try {
return listOperations.countGitHub({
// Awaited inside the try on purpose: returning the promise would let a per-repo
// rejection escape this catch, reject the whole batch and reach a caller with no
// handler. A failed count is a zero, not a failed load.
return await listOperations.countGitHub({
repoId: repo.id,
query: scopeGitHubTaskSearch(appliedQuery, githubKind)
})