Skip to content

Commit 3c7c016

Browse files
committed
Fix: pi-dispatch run must fail fast when Valkey is down, not hang forever
CI caught this as a hang; it is a real UX bug. The CLI enqueue used the same connection opts as the worker -- maxRetriesPerRequest: null -- so an unreachable Valkey made ioredis retry FOREVER. `pi-dispatch run` with Valkey down would hang indefinitely with no error. And the CLI test used port 6399 (my local test port) while CI's Valkey service is on 6379, so in CI the test's enqueue connected to nothing and hung until the job timed out. parseConnection now takes { failFast }: for the one-shot CLI producer it sets connectTimeout, enableOfflineQueue: false, and a retryStrategy that gives up after ~2 tries, so an enqueue against a down Valkey errors in ~1s with a clear "is it running? (docker compose up)" message. The long-running WORKER keeps the persistent default -- it should ride out a Valkey restart, not give up. Tests corrected: the real-Valkey enqueue now uses VALKEY_TEST_URL (skips without it, matching the queue integration test), and a new test asserts `run` against a DOWN Valkey returns 1 in under 15s -- the anti-hang guarantee, verified: it errored in 737ms and the runner exited cleanly.
1 parent d49b8a8 commit 3c7c016

3 files changed

Lines changed: 35 additions & 17 deletions

File tree

worker/src/cli.mjs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ export async function main(argv = process.argv.slice(2), env = process.env) {
5151
const config = loadConfig(env);
5252
const { parseConnection } = await import("./connection.mjs");
5353
const { makeQueue, enqueueLocalJob } = await import("./queue.mjs");
54-
const queue = makeQueue(parseConnection(config.valkeyUrl));
54+
// failFast: a one-shot enqueue must not hang forever if Valkey is down -- error clearly.
55+
const queue = makeQueue(parseConnection(config.valkeyUrl, { failFast: true }));
5556
try {
5657
const jobId = await enqueueLocalJob(queue, {
5758
folder,
@@ -62,8 +63,10 @@ export async function main(argv = process.argv.slice(2), env = process.env) {
6263
maxTurns: values["max-turns"] ? Number(values["max-turns"]) : config.maxTurns,
6364
});
6465
process.stdout.write(`queued ${jobId} — folder ${folder}\nrun \`pi-dispatch worker\` to process it.\n`);
66+
} catch (error) {
67+
return fail(`could not reach Valkey at ${config.valkeyUrl} — is it running? (docker compose up)\n ${error.message}`);
6568
} finally {
66-
await queue.close();
69+
await queue.close().catch(() => {});
6770
}
6871
return 0;
6972
}

worker/src/connection.mjs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,29 @@ import { Redis } from "ioredis";
88
* budget client, so it is set consistently.
99
*/
1010

11-
/** BullMQ connection options parsed from a redis:// URL. */
12-
export function parseConnection(url) {
11+
/**
12+
* BullMQ connection options parsed from a redis:// URL.
13+
*
14+
* `failFast` is for the CLI producer (a one-shot enqueue): if Valkey is unreachable it should
15+
* error in a couple of seconds with a clear message, not hang forever. The long-running WORKER
16+
* uses the default (persistent) options -- it should ride out a Valkey restart, not give up.
17+
*/
18+
export function parseConnection(url, { failFast = false } = {}) {
1319
const u = new URL(url);
1420
return {
1521
host: u.hostname || "127.0.0.1",
1622
port: Number(u.port || 6379),
1723
...(u.password ? { password: u.password } : {}),
1824
...(u.username ? { username: u.username } : {}),
1925
...(u.pathname && u.pathname !== "/" ? { db: Number(u.pathname.slice(1)) } : {}),
20-
maxRetriesPerRequest: null,
26+
maxRetriesPerRequest: null, // required for BullMQ blocking connections
27+
...(failFast
28+
? {
29+
connectTimeout: 2000,
30+
enableOfflineQueue: false, // don't buffer commands while disconnected -- error now
31+
retryStrategy: (attempts) => (attempts > 2 ? null : 200), // give up after ~2 tries
32+
}
33+
: {}),
2134
};
2235
}
2336

worker/test/cli.test.mjs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,18 @@ test("run refuses a dirty git working tree (edits are in place, no undo)", async
5353
assert.equal(await main(["run", dir, "--task", "x"], env), 1);
5454
});
5555

56-
test("--force overrides the dirty-tree refusal (reaches the enqueue, which needs Valkey)", async () => {
57-
// Without Valkey the enqueue will throw; we only assert the guard did NOT stop it (it got past
58-
// validation). If a Valkey is present it returns 0. Either way it must not return 1-from-guard.
59-
const dir = gitRepo({ dirty: true });
60-
let reachedEnqueue = false;
61-
try {
62-
const code = await main(["run", dir, "--task", "x", "--force"], env);
63-
reachedEnqueue = code === 0; // Valkey present
64-
} catch {
65-
reachedEnqueue = true; // tried to connect => passed the guard
66-
}
67-
assert.ok(reachedEnqueue, "--force must let a dirty tree through to the enqueue");
56+
test("run enqueues against a real Valkey (VALKEY_TEST_URL) and prints the job id", { skip: process.env.VALKEY_TEST_URL ? false : "needs VALKEY_TEST_URL" }, async () => {
57+
const dir = gitRepo({ dirty: false });
58+
const code = await main(["run", dir, "--task", "tidy the imports", "--force"], { VALKEY_URL: process.env.VALKEY_TEST_URL });
59+
assert.equal(code, 0, "a clean enqueue against a real Valkey returns 0");
60+
});
61+
62+
test("run fails FAST (does not hang) when Valkey is unreachable", async () => {
63+
// The whole point of failFast: a one-shot enqueue against a down Valkey must error in seconds,
64+
// not hang forever on ioredis's null retry policy. Port 1 is closed.
65+
const dir = gitRepo({ dirty: false });
66+
const start = Date.now();
67+
const code = await main(["run", dir, "--task", "x"], { VALKEY_URL: "redis://127.0.0.1:1" });
68+
assert.equal(code, 1, "an unreachable Valkey is a clean error, not a hang");
69+
assert.ok(Date.now() - start < 15000, "must fail fast, well under any CI timeout");
6870
});

0 commit comments

Comments
 (0)