fix(sandbox): improve docker image existence check error handling
Previously, `dockerImageExists` assumed any error from `docker image inspect` meant the image did not exist. This masked other errors like socket permission issues. This change: - Modifies `dockerImageExists` to inspect stderr when the exit code is non-zero. - Returns `false` only if the error explicitly indicates "No such image" or "No such object". - Throws an error with the stderr content for all other failures. - Adds a reproduction test in `src/agents/sandbox/docker.test.ts`.
This commit is contained in:
committed by
Peter Steinberger
parent
86db180a17
commit
49c6d8019f
44
src/agents/sandbox/docker.test.ts
Normal file
44
src/agents/sandbox/docker.test.ts
Normal file
@@ -0,0 +1,44 @@
|
|||||||
|
import { spawn } from "node:child_process";
|
||||||
|
import { describe, expect, it, vi, afterEach } from "vitest";
|
||||||
|
import { EventEmitter } from "events";
|
||||||
|
import { ensureDockerImage } from "./docker.js";
|
||||||
|
|
||||||
|
vi.mock("node:child_process", () => ({
|
||||||
|
spawn: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe("ensureDockerImage", () => {
|
||||||
|
afterEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
function mockSpawn(exitCode: number, stdout: string, stderr: string) {
|
||||||
|
const child = new EventEmitter() as any;
|
||||||
|
child.stdout = new EventEmitter();
|
||||||
|
child.stderr = new EventEmitter();
|
||||||
|
(spawn as any).mockReturnValue(child);
|
||||||
|
|
||||||
|
setTimeout(() => {
|
||||||
|
child.stdout.emit("data", Buffer.from(stdout));
|
||||||
|
child.stderr.emit("data", Buffer.from(stderr));
|
||||||
|
child.emit("close", exitCode);
|
||||||
|
}, 10);
|
||||||
|
return child;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("throws 'Sandbox image not found' when docker inspect fails with 'No such image'", async () => {
|
||||||
|
mockSpawn(1, "", "Error: No such image: test-image");
|
||||||
|
|
||||||
|
await expect(ensureDockerImage("test-image")).rejects.toThrow(
|
||||||
|
"Sandbox image not found: test-image. Build or pull it first."
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws 'Failed to inspect sandbox image' when docker inspect fails with other errors", async () => {
|
||||||
|
mockSpawn(1, "", "permission denied");
|
||||||
|
|
||||||
|
await expect(ensureDockerImage("test-image")).rejects.toThrow(
|
||||||
|
"Failed to inspect sandbox image: permission denied"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -50,7 +50,16 @@ async function dockerImageExists(image: string) {
|
|||||||
const result = await execDocker(["image", "inspect", image], {
|
const result = await execDocker(["image", "inspect", image], {
|
||||||
allowFailure: true,
|
allowFailure: true,
|
||||||
});
|
});
|
||||||
return result.code === 0;
|
if (result.code === 0) return true;
|
||||||
|
const stderr = result.stderr.trim();
|
||||||
|
if (
|
||||||
|
stderr.includes("No such image") ||
|
||||||
|
stderr.includes("No such object") ||
|
||||||
|
stderr.includes("not found")
|
||||||
|
) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
throw new Error(`Failed to inspect sandbox image: ${stderr}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function ensureDockerImage(image: string) {
|
export async function ensureDockerImage(image: string) {
|
||||||
|
|||||||
Reference in New Issue
Block a user