fix(win): record a redacted breadcrumb when fs:readDir throws (#6862)
The user report F0BDXH978GJ surfaced 'Error invoking remote method fs:readDir' — Electron's opaque message when the main-process handler throws. On Windows this is typically a WSL \wsl$ / \wsl.localhostUNC path or network drive failing realpath/readdir after the distro or share goes away, or a dropped SSH provider. Nothing recorded which throw site fired or what kind of path was involved, so the crash report carried no actionable cause. Wrap the fs:readDir handler to record an 'fs_readdir_error' breadcrumb on throw, tagging the throw site (ssh-provider | authorize | readdir), the error name/code, and a REDACTED path shape (isUNC, isWsl, driveLetter, hasConnectionId) — never the raw path. The error is re-thrown unchanged, so renderer behavior is identical; only crash reports gain structured context. Pure classification lives in readdir-error-diagnostics.ts (8 unit tests covering WSL UNC, \wsl$, network share, drive letter, SSH, and no-path-leak); 4 handler tests assert the breadcrumb fires per throw site and not on success. Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
parent
60412362a1
commit
9078ecaef5
|
|
@ -42,7 +42,8 @@ const {
|
|||
cancelGeneratePullRequestFieldsLocalMock,
|
||||
getSshFilesystemProviderMock,
|
||||
getSshGitProviderMock,
|
||||
tryDeleteWslUncPathMock
|
||||
tryDeleteWslUncPathMock,
|
||||
recordCrashBreadcrumbMock
|
||||
} = vi.hoisted(() => ({
|
||||
handleMock: vi.fn(),
|
||||
showSaveDialogMock: vi.fn(),
|
||||
|
|
@ -82,7 +83,8 @@ const {
|
|||
cancelGeneratePullRequestFieldsLocalMock: vi.fn(),
|
||||
getSshFilesystemProviderMock: vi.fn(),
|
||||
getSshGitProviderMock: vi.fn(),
|
||||
tryDeleteWslUncPathMock: vi.fn()
|
||||
tryDeleteWslUncPathMock: vi.fn(),
|
||||
recordCrashBreadcrumbMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
|
|
@ -116,6 +118,10 @@ vi.mock('../wsl-unc-delete', () => ({
|
|||
tryDeleteWslUncPath: tryDeleteWslUncPathMock
|
||||
}))
|
||||
|
||||
vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({
|
||||
recordCrashBreadcrumb: recordCrashBreadcrumbMock
|
||||
}))
|
||||
|
||||
vi.mock('../git/status', () => ({
|
||||
commitChanges: commitChangesMock,
|
||||
getStatus: getStatusMock,
|
||||
|
|
@ -247,6 +253,7 @@ describe('registerFilesystemHandlers', () => {
|
|||
rmMock,
|
||||
realpathMock,
|
||||
lstatMock,
|
||||
recordCrashBreadcrumbMock,
|
||||
commitChangesMock,
|
||||
getStatusMock,
|
||||
abortMergeMock,
|
||||
|
|
@ -325,6 +332,65 @@ describe('registerFilesystemHandlers', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('records a redacted breadcrumb when fs:readDir throws on a WSL UNC path', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
const wslPath = '\\\\wsl.localhost\\Ubuntu\\home\\user\\repo'
|
||||
// resolveAuthorizedPath authorizes the path, then readdir fails (distro stopped).
|
||||
realpathMock.mockResolvedValue(wslPath)
|
||||
registerWorktreeRootsForRepo(store as never, 'repo-1', [wslPath])
|
||||
readdirMock.mockRejectedValue(Object.assign(new Error('EIO: i/o error'), { code: 'EIO' }))
|
||||
|
||||
await expect(handlers.get('fs:readDir')!(null, { dirPath: wslPath })).rejects.toThrow(/EIO/)
|
||||
|
||||
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith('fs_readdir_error', {
|
||||
throwSite: 'readdir',
|
||||
errorName: 'Error',
|
||||
errorCode: 'EIO',
|
||||
hasConnectionId: false,
|
||||
isUNC: true,
|
||||
isWsl: true
|
||||
})
|
||||
// The raw path must never appear in the breadcrumb payload.
|
||||
const [, breadcrumbData] = recordCrashBreadcrumbMock.mock.calls[0]
|
||||
expect(JSON.stringify(breadcrumbData)).not.toContain('user')
|
||||
})
|
||||
|
||||
it('records a breadcrumb tagged ssh-provider when the SSH provider is gone', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
getSshFilesystemProviderMock.mockReturnValue(undefined)
|
||||
|
||||
await expect(
|
||||
handlers.get('fs:readDir')!(null, { dirPath: '/remote/repo', connectionId: 'ssh-1' })
|
||||
).rejects.toThrow()
|
||||
|
||||
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith(
|
||||
'fs_readdir_error',
|
||||
expect.objectContaining({ throwSite: 'ssh-provider', hasConnectionId: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('records a breadcrumb tagged authorize when the path is denied', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
await expect(
|
||||
handlers.get('fs:readDir')!(null, { dirPath: path.resolve('/etc/passwd') })
|
||||
).rejects.toThrow()
|
||||
|
||||
expect(recordCrashBreadcrumbMock).toHaveBeenCalledWith(
|
||||
'fs_readdir_error',
|
||||
expect.objectContaining({ throwSite: 'authorize', hasConnectionId: false })
|
||||
)
|
||||
})
|
||||
|
||||
it('does not record a breadcrumb when fs:readDir succeeds', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
readdirMock.mockResolvedValue([dirEntry({ name: 'file.ts', file: true })])
|
||||
|
||||
await handlers.get('fs:readDir')!(null, { dirPath: REPO_PATH })
|
||||
|
||||
expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects remote downloads with missing required arguments', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
|
|
|
|||
|
|
@ -115,6 +115,8 @@ import {
|
|||
type CommitMessageAgentEnvironmentResolvers
|
||||
} from '../text-generation/commit-message-agent-environment'
|
||||
import { listRepoWorktrees } from '../repo-worktrees'
|
||||
import { recordCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store'
|
||||
import { buildReadDirErrorBreadcrumb, type ReadDirThrowSite } from './readdir-error-diagnostics'
|
||||
import { splitWorktreeId } from '../../shared/worktree-id'
|
||||
import { getRuntimePathBasename } from '../../shared/cross-platform-path'
|
||||
import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options'
|
||||
|
|
@ -483,27 +485,48 @@ export function registerFilesystemHandlers(
|
|||
ipcMain.handle(
|
||||
'fs:readDir',
|
||||
async (_event, args: { dirPath: string; connectionId?: string }): Promise<DirEntry[]> => {
|
||||
if (args.connectionId) {
|
||||
const provider = requireSshFilesystemProvider(args.connectionId)
|
||||
return provider.readDir(args.dirPath)
|
||||
}
|
||||
const dirPath = await resolveAuthorizedPath(args.dirPath, store)
|
||||
const entries = await readdir(dirPath, { withFileTypes: true })
|
||||
const mapped = await Promise.all(
|
||||
entries.map(async (entry) => ({
|
||||
name: entry.name,
|
||||
isDirectory: await isDirectoryEntry(dirPath, entry, (entryPath) =>
|
||||
resolveAuthorizedPath(entryPath, store)
|
||||
),
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
)
|
||||
return mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
// Why: a thrown fs:readDir reaches the renderer as the opaque "Error
|
||||
// invoking remote method 'fs:readDir'" (Windows WSL/UNC realpath/readdir
|
||||
// failures, dropped SSH providers). Record which throw site fired plus a
|
||||
// redacted path shape so these are diagnosable without the raw path.
|
||||
let throwSite: ReadDirThrowSite = 'authorize'
|
||||
try {
|
||||
if (args.connectionId) {
|
||||
throwSite = 'ssh-provider'
|
||||
const provider = requireSshFilesystemProvider(args.connectionId)
|
||||
return await provider.readDir(args.dirPath)
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
throwSite = 'authorize'
|
||||
const dirPath = await resolveAuthorizedPath(args.dirPath, store)
|
||||
throwSite = 'readdir'
|
||||
const entries = await readdir(dirPath, { withFileTypes: true })
|
||||
const mapped = await Promise.all(
|
||||
entries.map(async (entry) => ({
|
||||
name: entry.name,
|
||||
isDirectory: await isDirectoryEntry(dirPath, entry, (entryPath) =>
|
||||
resolveAuthorizedPath(entryPath, store)
|
||||
),
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
)
|
||||
return mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
} catch (error: unknown) {
|
||||
recordCrashBreadcrumb(
|
||||
'fs_readdir_error',
|
||||
buildReadDirErrorBreadcrumb({
|
||||
dirPath: args.dirPath,
|
||||
connectionId: args.connectionId,
|
||||
throwSite,
|
||||
error
|
||||
})
|
||||
)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,69 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { buildReadDirErrorBreadcrumb, describeReadDirPathShape } from './readdir-error-diagnostics'
|
||||
|
||||
describe('describeReadDirPathShape', () => {
|
||||
it('classifies a WSL UNC path without leaking it', () => {
|
||||
const shape = describeReadDirPathShape('\\\\wsl.localhost\\Ubuntu\\home\\u\\repo', undefined)
|
||||
expect(shape).toEqual({ hasConnectionId: false, isUNC: true, isWsl: true })
|
||||
})
|
||||
|
||||
it('classifies the legacy \\\\wsl$ root as WSL', () => {
|
||||
expect(describeReadDirPathShape('\\\\wsl$\\Ubuntu\\home', undefined).isWsl).toBe(true)
|
||||
})
|
||||
|
||||
it('classifies a plain network UNC share as UNC but not WSL', () => {
|
||||
const shape = describeReadDirPathShape('\\\\fileserver\\share\\dir', undefined)
|
||||
expect(shape).toMatchObject({ isUNC: true, isWsl: false })
|
||||
expect(shape.driveLetter).toBeUndefined()
|
||||
})
|
||||
|
||||
it('extracts an uppercased drive letter for mapped drives', () => {
|
||||
expect(describeReadDirPathShape('z:\\projects\\repo', undefined)).toEqual({
|
||||
hasConnectionId: false,
|
||||
isUNC: false,
|
||||
isWsl: false,
|
||||
driveLetter: 'Z'
|
||||
})
|
||||
})
|
||||
|
||||
it('flags the SSH connection without recording it', () => {
|
||||
const shape = describeReadDirPathShape('/remote/repo', 'ssh-1')
|
||||
expect(shape).toEqual({ hasConnectionId: true, isUNC: false, isWsl: false })
|
||||
})
|
||||
|
||||
it('never includes the raw path in the shape', () => {
|
||||
const shape = describeReadDirPathShape('\\\\wsl.localhost\\Ubuntu\\secret\\path', 'ssh-9')
|
||||
expect(JSON.stringify(shape)).not.toContain('secret')
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildReadDirErrorBreadcrumb', () => {
|
||||
it('captures throw site, error code/name, and path shape', () => {
|
||||
const breadcrumb = buildReadDirErrorBreadcrumb({
|
||||
dirPath: '\\\\wsl.localhost\\Ubuntu\\home\\u\\repo',
|
||||
connectionId: undefined,
|
||||
throwSite: 'readdir',
|
||||
error: Object.assign(new Error('EIO: i/o error'), { code: 'EIO' })
|
||||
})
|
||||
expect(breadcrumb).toEqual({
|
||||
throwSite: 'readdir',
|
||||
errorName: 'Error',
|
||||
errorCode: 'EIO',
|
||||
hasConnectionId: false,
|
||||
isUNC: true,
|
||||
isWsl: true
|
||||
})
|
||||
})
|
||||
|
||||
it('omits errorCode when the error has none', () => {
|
||||
const breadcrumb = buildReadDirErrorBreadcrumb({
|
||||
dirPath: '/remote/repo',
|
||||
connectionId: 'ssh-1',
|
||||
throwSite: 'ssh-provider',
|
||||
error: new Error('Remote connection dropped.')
|
||||
})
|
||||
expect(breadcrumb).toMatchObject({ throwSite: 'ssh-provider', errorName: 'Error' })
|
||||
expect(breadcrumb.errorCode).toBeUndefined()
|
||||
expect(breadcrumb.hasConnectionId).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,57 @@
|
|||
import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting'
|
||||
|
||||
export type ReadDirThrowSite = 'ssh-provider' | 'authorize' | 'readdir'
|
||||
|
||||
/**
|
||||
* Redacted shape of a directory path for crash diagnostics.
|
||||
*
|
||||
* Why shape, not the path: "Error invoking remote method 'fs:readDir'" reaches
|
||||
* the renderer with no cause. We want to know *what kind* of path failed (WSL
|
||||
* UNC, network share, drive letter, SSH) without recording the path itself —
|
||||
* even though breadcrumbs are path-redacted downstream, never collecting the
|
||||
* raw path is the safer default.
|
||||
*/
|
||||
export function describeReadDirPathShape(
|
||||
dirPath: string,
|
||||
connectionId: string | undefined
|
||||
): CrashReportBreadcrumbData {
|
||||
const isUNC = /^[\\/]{2}/.test(dirPath)
|
||||
const lower = dirPath.toLowerCase()
|
||||
// \\wsl$\ and \\wsl.localhost\ (either slash direction) are WSL UNC roots.
|
||||
const isWsl = isUNC && (lower.includes('wsl$') || lower.includes('wsl.localhost'))
|
||||
const driveLetterMatch = /^([a-zA-Z]):[\\/]/.exec(dirPath)
|
||||
return {
|
||||
hasConnectionId: Boolean(connectionId),
|
||||
isUNC,
|
||||
isWsl,
|
||||
...(driveLetterMatch ? { driveLetter: driveLetterMatch[1].toUpperCase() } : {})
|
||||
}
|
||||
}
|
||||
|
||||
function errorCode(error: unknown): string | undefined {
|
||||
if (error && typeof error === 'object' && 'code' in error) {
|
||||
const code = (error as { code?: unknown }).code
|
||||
if (typeof code === 'string') {
|
||||
return code
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the redacted breadcrumb payload for a thrown fs:readDir call: which
|
||||
* throw site fired, the error code/name, and the path shape — no raw path.
|
||||
*/
|
||||
export function buildReadDirErrorBreadcrumb(args: {
|
||||
dirPath: string
|
||||
connectionId: string | undefined
|
||||
throwSite: ReadDirThrowSite
|
||||
error: unknown
|
||||
}): CrashReportBreadcrumbData {
|
||||
return {
|
||||
throwSite: args.throwSite,
|
||||
errorName: args.error instanceof Error ? args.error.name : typeof args.error,
|
||||
...(errorCode(args.error) ? { errorCode: errorCode(args.error)! } : {}),
|
||||
...describeReadDirPathShape(args.dirPath, args.connectionId)
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue