fix: address codex review round 1
Translate a mid-read AbortError from readFile into the seam's structured
FsError('FS_ABORTED') in readWholeText and readForEdit (the streaming/write
paths already did), and make the socket-type probe test reject on a listen
error instead of hanging where unix-domain sockets are unavailable.
This commit is contained in:
@@ -49,6 +49,22 @@ function throwIfAborted(signal: AbortSignal | undefined, verb: string): void {
|
||||
if (signal?.aborted) throw new FsError(`${verb} aborted`, 'FS_ABORTED')
|
||||
}
|
||||
|
||||
/**
|
||||
* `readFile` with the supplied signal, translating a mid-read `AbortError` into
|
||||
* the seam's structured `FsError('FS_ABORTED')` (Node rejects an aborted
|
||||
* `readFile` with a bare `AbortError`, which would otherwise escape the seam's
|
||||
* error taxonomy — the streaming/write paths translate it the same way).
|
||||
*/
|
||||
async function readFileAbortable(absolutePath: string, verb: 'read' | 'edit', signal?: AbortSignal): Promise<Buffer> {
|
||||
try {
|
||||
return await readFile(absolutePath, signal ? { signal } : {})
|
||||
} catch (error: unknown) {
|
||||
/* v8 ignore next 2 -- a non-abort readFile rejection needs a permission/IO fault racing an open file. */
|
||||
if (!isAbortError(error)) throw error
|
||||
throw new FsError(`${verb} aborted`, 'FS_ABORTED')
|
||||
}
|
||||
}
|
||||
|
||||
/** Opaque version token from a stat: mtime (ns precision) + size. */
|
||||
function versionOf(info: Stats): FsVersion {
|
||||
return FsVersion(`${info.mtimeMs}:${info.size}`)
|
||||
@@ -178,7 +194,7 @@ async function statRegularFile(target: LocalTarget, verb: 'read', signal?: Abort
|
||||
*/
|
||||
export async function readWholeText(target: LocalTarget, signal?: AbortSignal): Promise<string> {
|
||||
await statRegularFile(target, 'read', signal)
|
||||
const raw = await readFile(target.targetKey, signal ? { signal } : {})
|
||||
const raw = await readFileAbortable(target.targetKey, 'read', signal)
|
||||
throwIfAborted(signal, 'read')
|
||||
if (raw.subarray(0, BINARY_SAMPLE_BYTES).includes(0)) {
|
||||
throw new FsError(`cannot read "${target.displayPath}": binary file`, 'FS_NOT_TEXT')
|
||||
@@ -330,7 +346,7 @@ export async function readForEdit(
|
||||
signal?: AbortSignal,
|
||||
): Promise<{ content: string; lineEndings: LineEndings }> {
|
||||
throwIfAborted(signal, 'edit')
|
||||
const buffer = await readFile(absolutePath, signal ? { signal } : {})
|
||||
const buffer = await readFileAbortable(absolutePath, 'edit', signal)
|
||||
throwIfAborted(signal, 'edit')
|
||||
if (buffer.includes(0)) throw new FsError(`cannot edit "${displayPath}": binary file`, 'FS_NOT_TEXT')
|
||||
const raw = decodeUtf8(buffer, 'edit', displayPath)
|
||||
|
||||
@@ -94,7 +94,10 @@ describe('probe', () => {
|
||||
it('reports a socket/special file as type "other"', async () => {
|
||||
const sockPath = join(dir, 'sock')
|
||||
const server = createServer()
|
||||
await new Promise<void>((resolve) => { server.listen(sockPath, () => { resolve() }) })
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.once('error', reject)
|
||||
server.listen(sockPath, () => { resolve() })
|
||||
})
|
||||
try {
|
||||
expect((await probe(sockPath))?.type).toBe('other')
|
||||
} finally {
|
||||
@@ -133,6 +136,17 @@ describe('readWholeText', () => {
|
||||
await writeFile(file, 'one\ntwo')
|
||||
expect(await readWholeText(localTarget(file), new AbortController().signal)).toBe('one\ntwo')
|
||||
})
|
||||
|
||||
it('translates a mid-read AbortError into FS_ABORTED', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'one\ntwo')
|
||||
const ac = new AbortController()
|
||||
// Abort after the synchronous entry check but before readFile runs (the
|
||||
// stat await yields control back here), so readFile rejects AbortError.
|
||||
const pending = readWholeText(localTarget(file), ac.signal)
|
||||
ac.abort()
|
||||
await expect(pending).rejects.toMatchObject({ code: 'FS_ABORTED' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('streamWholeText', () => {
|
||||
@@ -281,4 +295,14 @@ describe('readForEdit + restoreLineEndings', () => {
|
||||
const original = await readForEdit(file, file, new AbortController().signal)
|
||||
expect(original.content).toBe('one\ntwo')
|
||||
})
|
||||
|
||||
it('translates a mid-read AbortError into FS_ABORTED', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'one\ntwo')
|
||||
const ac = new AbortController()
|
||||
// Abort after the synchronous entry check, while readFile is pending.
|
||||
const pending = readForEdit(file, file, ac.signal)
|
||||
ac.abort()
|
||||
await expect(pending).rejects.toMatchObject({ code: 'FS_ABORTED' })
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user