Bound concurrent repo scans and trace renderer breadcrumbs (#5608)
- Limit worktree refresh concurrency to 5 parallel workers to prevent launching too many concurrent git scans during startup and activation. - Record renderer breadcrumbs as tracer spans to provide durable pre-crash context for native main-process crashes. - Log a breadcrumb when activating a worktree from the sidebar.
This commit is contained in:
parent
03c18c4373
commit
f230c2d33d
|
|
@ -10,17 +10,32 @@ const {
|
|||
getDiagnosticsStatusMock,
|
||||
recordCrashBreadcrumbMock,
|
||||
resolveDiagnosticOrcaChannelMock,
|
||||
spanEndMock,
|
||||
startSpanMock,
|
||||
submitFeedbackMock
|
||||
} = vi.hoisted(() => ({
|
||||
handlers: new Map<string, (_event: unknown, args?: unknown) => unknown>(),
|
||||
listeners: new Map<string, (_event: unknown, args?: unknown) => void>(),
|
||||
clipboardWriteTextMock: vi.fn(),
|
||||
collectDiagnosticBundleMock: vi.fn(),
|
||||
getDiagnosticsStatusMock: vi.fn(),
|
||||
recordCrashBreadcrumbMock: vi.fn(),
|
||||
resolveDiagnosticOrcaChannelMock: vi.fn(),
|
||||
submitFeedbackMock: vi.fn()
|
||||
}))
|
||||
} = vi.hoisted(() => {
|
||||
const spanEndMock = vi.fn()
|
||||
return {
|
||||
handlers: new Map<string, (_event: unknown, args?: unknown) => unknown>(),
|
||||
listeners: new Map<string, (_event: unknown, args?: unknown) => void>(),
|
||||
clipboardWriteTextMock: vi.fn(),
|
||||
collectDiagnosticBundleMock: vi.fn(),
|
||||
getDiagnosticsStatusMock: vi.fn(),
|
||||
recordCrashBreadcrumbMock: vi.fn(),
|
||||
resolveDiagnosticOrcaChannelMock: vi.fn(),
|
||||
spanEndMock,
|
||||
startSpanMock: vi.fn(() => ({
|
||||
traceId: 'trace-id',
|
||||
spanId: 'span-id',
|
||||
setAttribute: vi.fn(),
|
||||
addEvent: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
interrupt: vi.fn(),
|
||||
end: spanEndMock
|
||||
})),
|
||||
submitFeedbackMock: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
app: { getVersion: () => '1.2.3-test' },
|
||||
|
|
@ -55,6 +70,10 @@ vi.mock('../observability/diagnostic-upload-endpoint', () => ({
|
|||
resolveDiagnosticOrcaChannel: resolveDiagnosticOrcaChannelMock
|
||||
}))
|
||||
|
||||
vi.mock('../observability/tracer', () => ({
|
||||
startSpan: startSpanMock
|
||||
}))
|
||||
|
||||
import {
|
||||
_getCrashReportingStateSizesForTests,
|
||||
_resetRendererErrorReportDedupeForTests,
|
||||
|
|
@ -110,6 +129,8 @@ describe('registerCrashReportingHandlers', () => {
|
|||
})
|
||||
resolveDiagnosticOrcaChannelMock.mockReset()
|
||||
resolveDiagnosticOrcaChannelMock.mockReturnValue('stable')
|
||||
startSpanMock.mockClear()
|
||||
spanEndMock.mockClear()
|
||||
submitFeedbackMock.mockReset()
|
||||
recordCrashBreadcrumbMock.mockReset()
|
||||
submitFeedbackMock.mockResolvedValue({ ok: true })
|
||||
|
|
@ -698,6 +719,19 @@ describe('registerCrashReportingHandlers', () => {
|
|||
ok: true,
|
||||
empty: null
|
||||
})
|
||||
expect(startSpanMock).toHaveBeenCalledWith('renderer.breadcrumb', {
|
||||
attributes: {
|
||||
kind: 'crash-breadcrumb',
|
||||
'breadcrumb.name': 'renderer_error',
|
||||
'breadcrumb.data': {
|
||||
message: 'boom',
|
||||
count: 2,
|
||||
ok: true,
|
||||
empty: null
|
||||
}
|
||||
}
|
||||
})
|
||||
expect(spanEndMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('ignores renderer breadcrumbs without a string name', () => {
|
||||
|
|
@ -717,5 +751,6 @@ describe('registerCrashReportingHandlers', () => {
|
|||
})
|
||||
|
||||
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
|
||||
expect(startSpanMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
type CrashReportSubmitResult,
|
||||
formatCrashReportText,
|
||||
formatUncapturedCrashReportText,
|
||||
sanitizeCrashReportDetails,
|
||||
sanitizeCrashReportString
|
||||
} from '../../shared/crash-reporting'
|
||||
import { submitFeedback } from './feedback'
|
||||
|
|
@ -21,6 +22,7 @@ import {
|
|||
} from '../crash-reporting/crash-breadcrumb-store'
|
||||
import { collectDiagnosticBundle, getDiagnosticsStatus } from '../observability'
|
||||
import { resolveDiagnosticOrcaChannel } from '../observability/diagnostic-upload-endpoint'
|
||||
import { startSpan } from '../observability/tracer'
|
||||
import type { FeedbackDiagnosticBundleAttachment } from './feedback'
|
||||
|
||||
const inFlightSubmissions = new Set<string>()
|
||||
|
|
@ -268,17 +270,34 @@ function sanitizeRendererBreadcrumbData(value: unknown): CrashReportBreadcrumbDa
|
|||
if (!value || typeof value !== 'object' || Array.isArray(value)) {
|
||||
return undefined
|
||||
}
|
||||
const sanitized: CrashReportBreadcrumbData = {}
|
||||
const primitiveData: Record<string, unknown> = {}
|
||||
for (const [key, entry] of Object.entries(value)) {
|
||||
if (typeof entry === 'string' || typeof entry === 'boolean' || entry === null) {
|
||||
sanitized[key] = entry
|
||||
primitiveData[key] = entry
|
||||
} else if (typeof entry === 'number' && Number.isFinite(entry)) {
|
||||
sanitized[key] = entry
|
||||
primitiveData[key] = entry
|
||||
}
|
||||
}
|
||||
const sanitized = sanitizeCrashReportDetails(primitiveData)
|
||||
return Object.keys(sanitized).length > 0 ? sanitized : undefined
|
||||
}
|
||||
|
||||
function recordRendererBreadcrumbTrace(
|
||||
name: string,
|
||||
data: CrashReportBreadcrumbData | undefined
|
||||
): void {
|
||||
const span = startSpan('renderer.breadcrumb', {
|
||||
attributes: {
|
||||
kind: 'crash-breadcrumb',
|
||||
'breadcrumb.name': sanitizeCrashReportString(name),
|
||||
...(data ? { 'breadcrumb.data': data } : {})
|
||||
}
|
||||
})
|
||||
// Why: main-process native crashes cannot persist memory-only breadcrumbs.
|
||||
// A tiny trace span gives the next crash report durable pre-crash context.
|
||||
span.end()
|
||||
}
|
||||
|
||||
function formatUnknownError(error: unknown): string {
|
||||
return sanitizeCrashReportString(error instanceof Error ? error.message : String(error))
|
||||
}
|
||||
|
|
@ -380,7 +399,9 @@ export function registerCrashReportingHandlers(store: CrashReportStore): void {
|
|||
if (!args || typeof args.name !== 'string') {
|
||||
return
|
||||
}
|
||||
recordCrashBreadcrumb(args.name, sanitizeRendererBreadcrumbData(args.data))
|
||||
const data = sanitizeRendererBreadcrumbData(args.data)
|
||||
recordCrashBreadcrumb(args.name, data)
|
||||
recordRendererBreadcrumbTrace(args.name, data)
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ import { DetachedHeadBadge } from '@/components/DetachedHeadBadge'
|
|||
import { getWorktreeGitIdentityDisplay } from '@/lib/worktree-git-identity-display'
|
||||
import { getFlushWorktreeCardPaddingLeft } from './worktree-list-indentation'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { recordRendererCrashBreadcrumb } from '@/lib/crash-diagnostics'
|
||||
import { folderWorkspaceKey, parseWorkspaceKey } from '../../../../shared/workspace-scope'
|
||||
|
||||
type WorktreeRenameRequest = {
|
||||
|
|
@ -578,6 +579,12 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
|||
// Why: route sidebar clicks through the shared activation path so the
|
||||
// back/forward stack stays complete for the primary worktree navigation
|
||||
// surface instead of only recording palette-driven switches.
|
||||
recordRendererCrashBreadcrumb('sidebar_worktree_activate', {
|
||||
worktreeId: worktree.id,
|
||||
repoId: worktree.repoId,
|
||||
wasActive: isActive,
|
||||
sshDisconnected: isSshDisconnected
|
||||
})
|
||||
onImmediateActivate?.(worktree.id, activationRowKey)
|
||||
activateWorktreeFromSidebar(worktree.id)
|
||||
if (isSshDisconnected) {
|
||||
|
|
@ -588,6 +595,8 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
|||
[
|
||||
affiliateListMode,
|
||||
worktree.id,
|
||||
worktree.repoId,
|
||||
isActive,
|
||||
isDeleting,
|
||||
activationRowKey,
|
||||
isSshDisconnected,
|
||||
|
|
|
|||
|
|
@ -3449,6 +3449,67 @@ describe('fetchAllWorktrees hydration-time purge (design §4.4)', () => {
|
|||
expect(store.getState().hasHydratedWorktreePurge).toBe(true)
|
||||
})
|
||||
|
||||
it('bounds concurrent repo scans during hydration-time refresh', async () => {
|
||||
const store = createTestStore()
|
||||
const repos = Array.from({ length: 7 }, (_, index) => ({
|
||||
id: `repo-${index}`,
|
||||
path: `/repos/${index}`,
|
||||
displayName: `repo-${index}`,
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}))
|
||||
let activeScans = 0
|
||||
let maxActiveScans = 0
|
||||
|
||||
mockApi.worktrees.list.mockImplementation(async ({ repoId }: { repoId: string }) => {
|
||||
activeScans += 1
|
||||
maxActiveScans = Math.max(maxActiveScans, activeScans)
|
||||
await new Promise((resolve) => setTimeout(resolve, 5))
|
||||
activeScans -= 1
|
||||
return [makeWorktree({ id: `${repoId}::/wt`, repoId, path: `/wt/${repoId}` })]
|
||||
})
|
||||
|
||||
store.setState({ repos } as unknown as Partial<AppState>)
|
||||
|
||||
await store.getState().fetchAllWorktrees()
|
||||
|
||||
expect(maxActiveScans).toBeLessThanOrEqual(5)
|
||||
expect(mockApi.worktrees.list).toHaveBeenCalledTimes(repos.length)
|
||||
expect(store.getState().hasHydratedWorktreePurge).toBe(true)
|
||||
})
|
||||
|
||||
it('bounds concurrent repo scans after the hydration purge has run', async () => {
|
||||
const store = createTestStore()
|
||||
const repos = Array.from({ length: 7 }, (_, index) => ({
|
||||
id: `repo-${index}`,
|
||||
path: `/repos/${index}`,
|
||||
displayName: `repo-${index}`,
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}))
|
||||
let activeScans = 0
|
||||
let maxActiveScans = 0
|
||||
|
||||
mockApi.worktrees.list.mockImplementation(async ({ repoId }: { repoId: string }) => {
|
||||
activeScans += 1
|
||||
maxActiveScans = Math.max(maxActiveScans, activeScans)
|
||||
await new Promise((resolve) => setTimeout(resolve, 5))
|
||||
activeScans -= 1
|
||||
return [makeWorktree({ id: `${repoId}::/wt`, repoId, path: `/wt/${repoId}` })]
|
||||
})
|
||||
|
||||
store.setState({
|
||||
hasHydratedWorktreePurge: true,
|
||||
repos
|
||||
} as unknown as Partial<AppState>)
|
||||
|
||||
await store.getState().fetchAllWorktrees()
|
||||
|
||||
expect(maxActiveScans).toBeLessThanOrEqual(5)
|
||||
expect(mockApi.worktrees.list).toHaveBeenCalledTimes(repos.length)
|
||||
expect(store.getState().hasHydratedWorktreePurge).toBe(true)
|
||||
})
|
||||
|
||||
it('preserves floating workspace state while purging a real stale worktree', async () => {
|
||||
const store = createTestStore()
|
||||
const wtA = makeWorktree({ id: 'repoA::/a/wt1', repoId: 'repoA', path: '/a/wt1' })
|
||||
|
|
|
|||
|
|
@ -63,10 +63,34 @@ const REMOTE_WORKTREE_LIST_PARITY_LIMIT = 10_000
|
|||
const ACTIVE_WORKTREE_TERMINAL_PREP_DELAY_MS = 300
|
||||
const ACTIVE_WORKTREE_TERMINAL_PREP_INPUT_QUIET_MS = 450
|
||||
const ACTIVE_WORKTREE_TERMINAL_PREP_IDLE_TIMEOUT_MS = 180
|
||||
const WORKTREE_REFRESH_CONCURRENCY = 5
|
||||
const pendingActivationTerminalPrepCancels = new Map<string, () => void>()
|
||||
const detachedHeadAutoDerivedDisplayNames = new Map<string, string>()
|
||||
const folderWorkspaceWorktreeCache = new WeakMap<FolderWorkspace, Worktree>()
|
||||
|
||||
async function mapReposForWorktreeRefresh<T>(
|
||||
repos: readonly { id: string }[],
|
||||
mapper: (repo: { id: string }) => Promise<T>
|
||||
): Promise<T[]> {
|
||||
const results = Array<T>(repos.length)
|
||||
let nextIndex = 0
|
||||
const workerCount = Math.min(WORKTREE_REFRESH_CONCURRENCY, repos.length)
|
||||
|
||||
// Why: worktree refresh can be triggered during activation/startup. Keeping
|
||||
// repo scans bounded avoids one UI moment launching every git probe at once.
|
||||
await Promise.all(
|
||||
Array.from({ length: workerCount }, async () => {
|
||||
while (nextIndex < repos.length) {
|
||||
const index = nextIndex
|
||||
nextIndex += 1
|
||||
results[index] = await mapper(repos[index])
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
return results
|
||||
}
|
||||
|
||||
function countTerminalLayoutLeaves(node: TerminalPaneLayoutNode | null | undefined): number {
|
||||
if (!node) {
|
||||
return 0
|
||||
|
|
@ -1293,7 +1317,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
// calls just need to refresh each repo's cached list. No need to
|
||||
// double-probe the IPC for the per-repo success signal.
|
||||
if (get().hasHydratedWorktreePurge) {
|
||||
await Promise.all(repos.map((r) => get().fetchWorktrees(r.id)))
|
||||
await mapReposForWorktreeRefresh(repos, (r) => get().fetchWorktrees(r.id))
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -1308,8 +1332,14 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
// empty-replace when cached data exists. Neither signal bubbles up to the
|
||||
// caller, so we probe the IPC directly to get the per-repo success signal,
|
||||
// then apply that same payload to state instead of listing each repo again.
|
||||
const results = await Promise.all(
|
||||
repos.map(async (r) => {
|
||||
const results = await mapReposForWorktreeRefresh(
|
||||
repos,
|
||||
async (
|
||||
r
|
||||
): Promise<
|
||||
| { repoId: string; ok: boolean; detected: DetectedWorktreeListResult }
|
||||
| { repoId: string; ok: false }
|
||||
> => {
|
||||
try {
|
||||
const detected = await listDetectedWorktreesForRepo(
|
||||
settingsForRepoOwner(get(), r.id),
|
||||
|
|
@ -1336,7 +1366,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
console.error(`Failed to fetch worktrees for repo ${r.id}:`, err)
|
||||
return { repoId: r.id, ok: false as const }
|
||||
}
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
const hasAnyDetectedWorktree = results.some(
|
||||
|
|
|
|||
Loading…
Reference in New Issue