feat(shared): wire tolerantFsync to record skips with path classification
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -1,7 +1,8 @@
|
|||||||
import { describe, expect, it } from "bun:test"
|
import { beforeEach, describe, expect, it } from "bun:test"
|
||||||
import { fsyncSync } from "node:fs"
|
import { fsyncSync } from "node:fs"
|
||||||
import type { FileHandle } from "node:fs/promises"
|
import type { FileHandle } from "node:fs/promises"
|
||||||
|
|
||||||
|
import { clearAllSkips, drainSkipsAfter } from "./fsync-skip-tracker"
|
||||||
import { isToleratedFsyncError, tolerantFsync, tolerantFsyncSync } from "./tolerant-fsync"
|
import { isToleratedFsyncError, tolerantFsync, tolerantFsyncSync } from "./tolerant-fsync"
|
||||||
|
|
||||||
function makeFsError(code: string, message?: string): NodeJS.ErrnoException {
|
function makeFsError(code: string, message?: string): NodeJS.ErrnoException {
|
||||||
@@ -60,6 +61,10 @@ describe("isToleratedFsyncError", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("tolerantFsync (async)", () => {
|
describe("tolerantFsync (async)", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
clearAllSkips()
|
||||||
|
})
|
||||||
|
|
||||||
it("#given fsync throws EPERM #when called #then resolves without throwing", async () => {
|
it("#given fsync throws EPERM #when called #then resolves without throwing", async () => {
|
||||||
const handle = fakeHandleWithSyncError(makeFsError("EPERM", "operation not permitted, fsync"))
|
const handle = fakeHandleWithSyncError(makeFsError("EPERM", "operation not permitted, fsync"))
|
||||||
await expect(tolerantFsync(handle, "test:async-eperm")).resolves.toBeUndefined()
|
await expect(tolerantFsync(handle, "test:async-eperm")).resolves.toBeUndefined()
|
||||||
@@ -100,6 +105,24 @@ describe("tolerantFsync (async)", () => {
|
|||||||
await tolerantFsync(handle, "test:async-success")
|
await tolerantFsync(handle, "test:async-success")
|
||||||
expect(syncCalled).toBe(true)
|
expect(syncCalled).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("#given fsync throws EPERM #when called #then tracker records one skip", async () => {
|
||||||
|
const handle = fakeHandleWithSyncError(makeFsError("EPERM", "operation not permitted, fsync"))
|
||||||
|
|
||||||
|
await tolerantFsync(handle, "atomicWrite:/Users/x/Library/Mobile Documents/com~apple~CloudDocs/file.txt")
|
||||||
|
|
||||||
|
const entries = drainSkipsAfter(0)
|
||||||
|
expect(entries).toHaveLength(1)
|
||||||
|
expect(entries[0]?.errorCode).toBe("EPERM")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#given fsync throws EIO #when called #then tracker remains empty", async () => {
|
||||||
|
const handle = fakeHandleWithSyncError(makeFsError("EIO"))
|
||||||
|
|
||||||
|
await expect(tolerantFsync(handle, "atomicWrite:/tmp/file.txt")).rejects.toThrow("EIO: simulated")
|
||||||
|
|
||||||
|
expect(drainSkipsAfter(0)).toHaveLength(0)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("tolerantFsyncSync (synchronous)", () => {
|
describe("tolerantFsyncSync (synchronous)", () => {
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import { fsyncSync } from "node:fs"
|
import { fsyncSync } from "node:fs"
|
||||||
import type { FileHandle } from "node:fs/promises"
|
import type { FileHandle } from "node:fs/promises"
|
||||||
|
|
||||||
|
import { classifyPathEnvironment } from "./classify-path-environment"
|
||||||
|
import { recordFsyncSkip } from "./fsync-skip-tracker"
|
||||||
import { log } from "./logger"
|
import { log } from "./logger"
|
||||||
|
|
||||||
const TOLERATED_FSYNC_CODES: ReadonlySet<string> = new Set([
|
const TOLERATED_FSYNC_CODES: ReadonlySet<string> = new Set([
|
||||||
@@ -16,6 +18,13 @@ export function isToleratedFsyncError(error: unknown): boolean {
|
|||||||
return code !== undefined && TOLERATED_FSYNC_CODES.has(code)
|
return code !== undefined && TOLERATED_FSYNC_CODES.has(code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function extractPathFromContextLabel(contextLabel: string): string {
|
||||||
|
const separatorIndex = contextLabel.indexOf(":")
|
||||||
|
if (separatorIndex < 0) return contextLabel
|
||||||
|
|
||||||
|
return contextLabel.slice(separatorIndex + 1)
|
||||||
|
}
|
||||||
|
|
||||||
export async function tolerantFsync(
|
export async function tolerantFsync(
|
||||||
fileHandle: FileHandle,
|
fileHandle: FileHandle,
|
||||||
contextLabel: string,
|
contextLabel: string,
|
||||||
@@ -24,11 +33,23 @@ export async function tolerantFsync(
|
|||||||
await fileHandle.sync()
|
await fileHandle.sync()
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (!isToleratedFsyncError(error)) throw error
|
if (!isToleratedFsyncError(error)) throw error
|
||||||
|
const errorCode = (error as NodeJS.ErrnoException).code ?? "UNKNOWN"
|
||||||
|
const message = error instanceof Error ? error.message : String(error)
|
||||||
|
const filePath = extractPathFromContextLabel(contextLabel)
|
||||||
|
|
||||||
log("fsync skipped due to filesystem limitation", {
|
log("fsync skipped due to filesystem limitation", {
|
||||||
event: "fsync-skipped",
|
event: "fsync-skipped",
|
||||||
contextLabel,
|
contextLabel,
|
||||||
code: (error as NodeJS.ErrnoException).code,
|
code: errorCode,
|
||||||
message: error instanceof Error ? error.message : String(error),
|
message,
|
||||||
|
})
|
||||||
|
|
||||||
|
recordFsyncSkip({
|
||||||
|
filePath,
|
||||||
|
contextLabel,
|
||||||
|
errorCode,
|
||||||
|
message,
|
||||||
|
pathClassification: classifyPathEnvironment(filePath),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -42,11 +63,23 @@ export function tolerantFsyncSync(
|
|||||||
fsyncImpl(fileDescriptor)
|
fsyncImpl(fileDescriptor)
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (!isToleratedFsyncError(error)) throw error
|
if (!isToleratedFsyncError(error)) throw error
|
||||||
|
const errorCode = (error as NodeJS.ErrnoException).code ?? "UNKNOWN"
|
||||||
|
const message = error instanceof Error ? error.message : String(error)
|
||||||
|
const filePath = extractPathFromContextLabel(contextLabel)
|
||||||
|
|
||||||
log("fsync skipped due to filesystem limitation", {
|
log("fsync skipped due to filesystem limitation", {
|
||||||
event: "fsync-skipped",
|
event: "fsync-skipped",
|
||||||
contextLabel,
|
contextLabel,
|
||||||
code: (error as NodeJS.ErrnoException).code,
|
code: errorCode,
|
||||||
message: error instanceof Error ? error.message : String(error),
|
message,
|
||||||
|
})
|
||||||
|
|
||||||
|
recordFsyncSkip({
|
||||||
|
filePath,
|
||||||
|
contextLabel,
|
||||||
|
errorCode,
|
||||||
|
message,
|
||||||
|
pathClassification: classifyPathEnvironment(filePath),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user