diff --git a/eslint-plugins/healthlog/__tests__/job-handler-outcome.test.mjs b/eslint-plugins/healthlog/__tests__/job-handler-outcome.test.mjs new file mode 100644 index 000000000..7d608ec10 --- /dev/null +++ b/eslint-plugins/healthlog/__tests__/job-handler-outcome.test.mjs @@ -0,0 +1,67 @@ +/** + * Unit tests for the `healthlog/job-handler-outcome` rule — the bare + * `boss.work(` ban, the one exempt module that owns `createAndWork`, the + * test-file exemption, and the scope limit to `src/lib/jobs/`. + * + * The `invalid` cases below also stand as the rule's own documented blind + * spots: an aliased or renamed receiver is not detected, and neither is a + * handler that returns `{ ok: true }` after swallowing. Those sit in `valid` + * on purpose, so a future reader can see what this rule does not claim. + */ +import { describe, it } from "vitest"; +import { RuleTester } from "eslint"; +import rule from "../job-handler-outcome.js"; + +RuleTester.describe = describe; +RuleTester.it = it; +RuleTester.itOnly = it.only; + +const ruleTester = new RuleTester({ + languageOptions: { ecmaVersion: 2023, sourceType: "module" }, +}); + +ruleTester.run("job-handler-outcome", rule, { + valid: [ + { + code: "await createAndWork(boss, QUEUE, { localConcurrency: 1 }, handleX);", + filename: "/repo/src/lib/jobs/reminder/register-status.ts", + }, + { + // The wrapper's own call — this module is the one legal caller. + code: "await boss.work(queue, options, runJob(queue, handler));", + filename: "/repo/src/lib/jobs/reminder/registrar-shared.ts", + }, + { + // Tests mock `boss.work` and assert against it directly. + code: 'boss.work("q", {}, async () => {});', + filename: "/repo/src/lib/jobs/__tests__/whoop-queues.test.ts", + }, + { + // Out of scope: not the worker tree. + code: 'boss.work("q", {}, async () => {});', + filename: "/repo/src/lib/integrations/status.ts", + }, + { + // Documented blind spot: an aliased receiver is not detected. + code: 'const b = boss; b.work("q", {}, async () => {});', + filename: "/repo/src/lib/jobs/reminder/register-rollup.ts", + }, + { + // Documented blind spot: this rule cannot see a swallowed failure. + code: "await createAndWork(boss, QUEUE, {}, async () => ({ ok: true, did: {} }));", + filename: "/repo/src/lib/jobs/reminder/register-rollup.ts", + }, + ], + invalid: [ + { + code: 'await boss.work("scratch", async () => {});', + filename: "/repo/src/lib/jobs/reminder/register-maintenance.ts", + errors: [{ messageId: "bareBossWork" }], + }, + { + code: "await boss.work(QUEUE, { localConcurrency: 1 }, handleX);", + filename: "/repo/src/lib/jobs/worker-status.ts", + errors: [{ messageId: "bareBossWork" }], + }, + ], +}); diff --git a/src/lib/jobs/__tests__/run-job.test.ts b/src/lib/jobs/__tests__/run-job.test.ts new file mode 100644 index 000000000..6973cb89a --- /dev/null +++ b/src/lib/jobs/__tests__/run-job.test.ts @@ -0,0 +1,112 @@ +/** + * Unit tests for the job-outcome spine. + * + * The whole point of `runJob` is that an `ok: false` outcome reaches pg-boss + * as a failed job. That is a behaviour claim, not a typing one, and it is the + * claim the retention cleanups now depend on — so it is pinned here: the + * failure is reported once with the handler's own facts, it rethrows, and a + * handler that throws on its own is passed through untouched rather than + * rewrapped into a stack that points at this file. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { Job } from "pg-boss"; + +const reportWorkerError = vi.fn(); + +vi.mock("../report-worker-error", () => ({ + reportWorkerError: (...a: unknown[]) => reportWorkerError(...a), +})); + +import { jobDone, jobFailed } from "../job-outcome"; +import { JobFailure, runJob } from "../run-job"; + +const noJobs: Job[] = []; + +beforeEach(() => { + vi.clearAllMocks(); + reportWorkerError.mockResolvedValue(undefined); +}); + +describe("runJob", () => { + it("resolves and reports nothing when the handler says it did the work", async () => { + const wrapped = runJob("rate-limit-cleanup", async () => + jobDone({ deleted: 12 }), + ); + + await expect(wrapped(noJobs)).resolves.toBeUndefined(); + expect(reportWorkerError).not.toHaveBeenCalled(); + }); + + it("treats a run with nothing to do as a success", async () => { + const wrapped = runJob("mood-reminder-cleanup", async () => + jobDone({ deleted: 0 }), + ); + + await expect(wrapped(noJobs)).resolves.toBeUndefined(); + expect(reportWorkerError).not.toHaveBeenCalled(); + }); + + it("rethrows a failed outcome so pg-boss records a failed job", async () => { + const cause = new Error("statement timeout"); + const wrapped = runJob("push-attempt-cleanup", async () => + jobFailed("push-attempt cleanup failed", cause, { deleted: 0 }), + ); + + await expect(wrapped(noJobs)).rejects.toBeInstanceOf(JobFailure); + await expect(wrapped(noJobs)).rejects.toThrow( + "push-attempt-cleanup: push-attempt cleanup failed", + ); + }); + + it("reports the failure with the handler's own facts and the cause", async () => { + const cause = new Error("statement timeout"); + const wrapped = runJob("push-attempt-cleanup", async () => + jobFailed("push-attempt cleanup failed", cause, { deleted: 0 }), + ); + + await expect(wrapped(noJobs)).rejects.toThrow(); + + expect(reportWorkerError).toHaveBeenCalledTimes(1); + const [queue, error, meta] = reportWorkerError.mock.calls[0]; + expect(queue).toBe("push-attempt-cleanup"); + expect(error).toBeInstanceOf(JobFailure); + expect(meta).toEqual({ deleted: 0, cause: "statement timeout" }); + }); + + it("carries the thrown error's stack instead of its own frame", async () => { + const cause = new Error("statement timeout"); + cause.stack = "Error: statement timeout\n at deleteMany (prisma.ts:1:1)"; + const wrapped = runJob("mcp-token-cleanup", async () => + jobFailed("mcp-token cleanup failed", cause), + ); + + const failure = await wrapped(noJobs).catch((err: unknown) => err); + expect(failure).toBeInstanceOf(JobFailure); + expect((failure as JobFailure).stack).toContain("at deleteMany"); + expect((failure as JobFailure).cause).toBe(cause); + }); + + it("passes a throwing handler through untouched", async () => { + const thrown = new Error("cohort discovery failed"); + const wrapped = runJob("withings-sync", async () => { + throw thrown; + }); + + await expect(wrapped(noJobs)).rejects.toBe(thrown); + // The throw already fails the job. Reporting it here would double every + // handler that reports for itself, and rewrapping would bury the stack. + expect(reportWorkerError).not.toHaveBeenCalled(); + }); + + it("hands the batch to the handler unchanged", async () => { + const handler = vi.fn(async () => jobDone({ jobs: 2 })); + const jobs = [ + { id: "a", data: { userId: "u1" } }, + { id: "b", data: { userId: "u2" } }, + ] as unknown as Job<{ userId: string }>[]; + + await runJob("data-arrival", handler)(jobs); + + expect(handler).toHaveBeenCalledWith(jobs); + }); +});