mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 11:25:50 -06:00
fix: heartbeat survives transient filesystem read races (#100389)
* fix(heartbeat): retry transient workspace reads
* fix(heartbeat): bound transient filesystem retries
* fix(sessions): stop retrying permanent read failures
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
(cherry picked from commit 92af54b64f)
This commit is contained in:
committed by
Dallin Romney
parent
2166bc1c8c
commit
aaa2c95c32
@@ -635,6 +635,64 @@ describe("ensureAgentWorkspace", () => {
|
||||
await expect(isWorkspaceBootstrapPending(tempDir)).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it("retries transient profile reads before stale bootstrap repair", async () => {
|
||||
const tempDir = await makeTempWorkspace("openclaw-workspace-");
|
||||
await ensureAgentWorkspace({ dir: tempDir, ensureBootstrapFiles: true });
|
||||
await writeWorkspaceFile({
|
||||
dir: tempDir,
|
||||
name: DEFAULT_IDENTITY_FILENAME,
|
||||
content: "# IDENTITY.md\n\n- **Name:** Example\n",
|
||||
});
|
||||
|
||||
const identityPath = path.join(tempDir, DEFAULT_IDENTITY_FILENAME);
|
||||
const originalReadFile = fs.readFile.bind(fs);
|
||||
let identityReads = 0;
|
||||
const readSpy = vi.spyOn(fs, "readFile").mockImplementation((async (filePath, options) => {
|
||||
if (filePath === identityPath) {
|
||||
identityReads += 1;
|
||||
if (identityReads === 1) {
|
||||
throw Object.assign(
|
||||
new Error("Unknown system error -11: Unknown system error -11, read"),
|
||||
{ code: "EAGAIN", errno: -11 },
|
||||
);
|
||||
}
|
||||
}
|
||||
return await originalReadFile(filePath, options as never);
|
||||
}) as typeof fs.readFile);
|
||||
|
||||
try {
|
||||
await ensureAgentWorkspace({ dir: tempDir, ensureBootstrapFiles: true });
|
||||
expect(identityReads).toBeGreaterThanOrEqual(2);
|
||||
await expectPathMissing(path.join(tempDir, DEFAULT_BOOTSTRAP_FILENAME));
|
||||
} finally {
|
||||
readSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("propagates a transient profile read after the retry budget is exhausted", async () => {
|
||||
const tempDir = await makeTempWorkspace("openclaw-workspace-");
|
||||
await ensureAgentWorkspace({ dir: tempDir, ensureBootstrapFiles: true });
|
||||
const identityPath = path.join(tempDir, DEFAULT_IDENTITY_FILENAME);
|
||||
const originalReadFile = fs.readFile.bind(fs);
|
||||
const readSpy = vi.spyOn(fs, "readFile").mockImplementation((async (filePath, options) => {
|
||||
if (filePath === identityPath) {
|
||||
throw Object.assign(new Error("Unknown system error -11, read"), {
|
||||
code: "EAGAIN",
|
||||
errno: -11,
|
||||
});
|
||||
}
|
||||
return await originalReadFile(filePath, options as never);
|
||||
}) as typeof fs.readFile);
|
||||
|
||||
try {
|
||||
await expect(
|
||||
ensureAgentWorkspace({ dir: tempDir, ensureBootstrapFiles: true }),
|
||||
).rejects.toMatchObject({ code: "EAGAIN" });
|
||||
} finally {
|
||||
readSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("records stale bootstrap completion when BOOTSTRAP.md cleanup fails", async () => {
|
||||
const tempDir = await makeTempWorkspace("openclaw-workspace-");
|
||||
await ensureAgentWorkspace({ dir: tempDir, ensureBootstrapFiles: true });
|
||||
|
||||
+24
-4
@@ -12,6 +12,7 @@ import { resolveLegacyStateDirs, resolveStateDir } from "../config/paths.js";
|
||||
import { openRootFile } from "../infra/boundary-file-read.js";
|
||||
import { pathExists } from "../infra/fs-safe.js";
|
||||
import { replaceFileAtomic } from "../infra/replace-file.js";
|
||||
import { retryAsync } from "../infra/retry.js";
|
||||
import {
|
||||
CANONICAL_ROOT_MEMORY_FILENAME,
|
||||
exactWorkspaceEntryExists,
|
||||
@@ -50,6 +51,9 @@ const WORKSPACE_ONBOARDING_PROFILE_FILENAMES = [
|
||||
DEFAULT_IDENTITY_FILENAME,
|
||||
DEFAULT_USER_FILENAME,
|
||||
] as const;
|
||||
const TRANSIENT_WORKSPACE_READ_CODES = new Set(["EAGAIN", "EWOULDBLOCK", "EINTR"]);
|
||||
const TRANSIENT_WORKSPACE_READ_ERRNOS = new Set([-11, -4]);
|
||||
const TRANSIENT_WORKSPACE_READ_MESSAGE = /Unknown system error -(?:11|4)\b/i;
|
||||
|
||||
const workspaceTemplateCache = new Map<string, Promise<string>>();
|
||||
let gitAvailabilityPromise: Promise<boolean> | null = null;
|
||||
@@ -245,18 +249,34 @@ async function writeFileIfMissing(filePath: string, content: string): Promise<bo
|
||||
}
|
||||
}
|
||||
|
||||
function isTransientWorkspaceReadError(error: unknown): boolean {
|
||||
const fsError = error as NodeJS.ErrnoException | undefined;
|
||||
if (fsError?.code && TRANSIENT_WORKSPACE_READ_CODES.has(fsError.code)) {
|
||||
return true;
|
||||
}
|
||||
if (typeof fsError?.errno === "number" && TRANSIENT_WORKSPACE_READ_ERRNOS.has(fsError.errno)) {
|
||||
return true;
|
||||
}
|
||||
return error instanceof Error && TRANSIENT_WORKSPACE_READ_MESSAGE.test(error.message);
|
||||
}
|
||||
|
||||
async function fileContentDiffersFromTemplate(
|
||||
filePath: string,
|
||||
template: string,
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
return (await fs.readFile(filePath, "utf-8")) !== template;
|
||||
return await retryAsync(async () => (await fs.readFile(filePath, "utf-8")) !== template, {
|
||||
attempts: 3,
|
||||
minDelayMs: 50,
|
||||
maxDelayMs: 50,
|
||||
shouldRetry: (err) => isTransientWorkspaceReadError(err),
|
||||
});
|
||||
} catch (err) {
|
||||
const anyErr = err as { code?: string };
|
||||
if (anyErr.code !== "ENOENT") {
|
||||
throw err;
|
||||
if (anyErr.code === "ENOENT") {
|
||||
return false;
|
||||
}
|
||||
return false;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -135,6 +135,47 @@ describe("Session Store Cache", () => {
|
||||
expect(loaded).toEqual(testStore);
|
||||
});
|
||||
|
||||
it("retries transient session store read failures", async () => {
|
||||
const testStore = createSingleSessionStore();
|
||||
await saveSessionStore(storePath, testStore);
|
||||
clearSessionStoreCacheForTest();
|
||||
|
||||
const originalReadFileSync = fs.readFileSync.bind(fs);
|
||||
let storeReads = 0;
|
||||
const readSpy = vi.spyOn(fs, "readFileSync").mockImplementation((file, ...args) => {
|
||||
if (file === storePath) {
|
||||
storeReads += 1;
|
||||
if (storeReads === 1) {
|
||||
throw Object.assign(
|
||||
new Error("Unknown system error -11: Unknown system error -11, read"),
|
||||
{ code: "EAGAIN", errno: -11 },
|
||||
);
|
||||
}
|
||||
}
|
||||
return originalReadFileSync(file, ...(args as [Parameters<typeof fs.readFileSync>[1]]));
|
||||
});
|
||||
|
||||
try {
|
||||
expect(loadSessionStore(storePath, { skipCache: true })).toEqual(testStore);
|
||||
expect(storeReads).toBe(2);
|
||||
} finally {
|
||||
readSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not retry permanent session store read failures", () => {
|
||||
clearSessionStoreCacheForTest();
|
||||
const missingPath = path.join(testDir, "missing-sessions.json");
|
||||
const readSpy = vi.spyOn(fs, "readFileSync");
|
||||
|
||||
try {
|
||||
expect(loadSessionStore(missingPath, { skipCache: true })).toEqual({});
|
||||
expect(readSpy).toHaveBeenCalledOnce();
|
||||
} finally {
|
||||
readSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("should serve freshly saved session stores from cache without disk reads", async () => {
|
||||
const testStore = createSingleSessionStore();
|
||||
|
||||
|
||||
@@ -392,13 +392,13 @@ export function loadSessionStore(
|
||||
}
|
||||
}
|
||||
|
||||
// Retry a few times on Windows because readers can briefly observe empty or
|
||||
// Retry a few times because readers can briefly observe empty or
|
||||
// transiently invalid content while another process is swapping the file.
|
||||
let store: Record<string, SessionEntry> = {};
|
||||
const fileStat = getFileStatSnapshot(storePath);
|
||||
const mtimeMs = fileStat?.mtimeMs;
|
||||
let serializedFromDisk: string | undefined;
|
||||
const maxReadAttempts = process.platform === "win32" ? 3 : 1;
|
||||
const maxReadAttempts = 3;
|
||||
const retryBuf = maxReadAttempts > 1 ? new Int32Array(new SharedArrayBuffer(4)) : undefined;
|
||||
for (let attempt = 0; attempt < maxReadAttempts; attempt += 1) {
|
||||
try {
|
||||
@@ -416,7 +416,12 @@ export function loadSessionStore(
|
||||
// writes the file after readFileSync returns, a post-read stat could tag
|
||||
// stale content as current and make future cache hits return old data.
|
||||
break;
|
||||
} catch {
|
||||
} catch (err) {
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
const isPermanentReadError = code === "ENOENT" || code === "EACCES" || code === "EPERM";
|
||||
if (isPermanentReadError) {
|
||||
break;
|
||||
}
|
||||
if (attempt < maxReadAttempts - 1) {
|
||||
Atomics.wait(retryBuf!, 0, 0, 50);
|
||||
continue;
|
||||
|
||||
Reference in New Issue
Block a user