Skip to content

Commit 557a8af

Browse files
authored
fix(service-job,service-automation): map a degraded job outcome to sys_job_run.status instead of success (#5548) (#7446)
* fix(service-job,service-automation): map a degraded job outcome to sys_job_run.status instead of success (#5548) * test(service-job,service-automation): bind the two new engine doubles to assertEngineUpdateDispatch (#5548)
1 parent 24d22f4 commit 557a8af

10 files changed

Lines changed: 617 additions & 11 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/service-job': patch
3+
'@objectstack/service-automation': patch
4+
---
5+
6+
Job runs that finish without doing their work are now audited as `degraded`, not `success` (#5548)
7+
8+
`DbJobAdapter` decided a run's outcome solely by whether the handler threw, so a
9+
handler that failed internally and deliberately did not throw was recorded as
10+
`sys_job_run.status: 'success'` — the audit surface Studio's jobs view reads
11+
reported the one thing that had definitely not happened.
12+
13+
The adapters now consume the `JobRunOutcome` channel `JobHandler` gained in
14+
#6617, using the `degraded` status vocabulary added in #7072:
15+
16+
- a handler resolving `{ outcome: 'degraded', reason? }` lands
17+
`sys_job_run.status: 'degraded'` with the reason in `error`, and mirrors onto
18+
`sys_job.last_status` / `last_error`;
19+
- `degraded` is not a failure: `failure_count` stays flat and nothing retries
20+
(retry keys on a rejected promise only, unchanged);
21+
- `IntervalJobAdapter` / `CronJobAdapter` report the same verdict through
22+
`getExecutions()`, so the in-memory history and the persisted row agree.
23+
24+
Strictly additive: a handler that resolves `undefined` — every handler written
25+
before #6617 — is still recorded as `success`, byte for byte as before.
26+
27+
The first adopter is the `wait` node's timer wake-up: a shot that fires into an
28+
unreachable suspended-run store now reports `degraded` / `STORE_UNAVAILABLE`
29+
while still keeping its one-shot armed and its `sys_job` row active (#5529).

packages/services/service-automation/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@
2424
},
2525
"devDependencies": {
2626
"@objectstack/driver-sql": "workspace:*",
27+
"@objectstack/metadata-core": "workspace:*",
2728
"@objectstack/objectql": "workspace:*",
2829
"@objectstack/plugin-security": "workspace:*",
30+
"@objectstack/service-job": "workspace:*",
2931
"@types/node": "^26.1.2",
3032
"typescript": "^6.0.3",
3133
"vitest": "^4.1.10"
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
5+
import { DbJobAdapter } from '@objectstack/service-job';
6+
import type { IJobService, JobSchedule, JobHandler } from '@objectstack/spec/contracts';
7+
import { AutomationEngine } from '../engine.js';
8+
import type { NodeExecutor } from '../engine.js';
9+
import { InMemorySuspendedRunStore } from '../suspended-run-store.js';
10+
import { registerWaitNode, rearmSuspendedWaitTimers } from './wait-node.js';
11+
12+
/**
13+
* #5548, end to end: the scenario that produced the finding, driven through the
14+
* REAL job adapter rather than a fake that records calls.
15+
*
16+
* The specimen is #5529's wait wake-up firing into an unreachable durable store.
17+
* That shot consumes nothing — the run stays parked, and the one-shot is kept
18+
* ARMED on purpose so it can be re-fired — and it deliberately does **not**
19+
* throw, because a throw is the retry signal `IJobService` implementations key
20+
* on (which is why option A was rejected). The consequence, until now, was that
21+
* the job's audit row said `success`: the operator-facing surface reported the
22+
* one thing that definitely did not happen.
23+
*
24+
* Why the real `DbJobAdapter` and not a spy: the defect lives in the mapping
25+
* from "what the handler reported" to "what got written", so a case that
26+
* asserts the handler was called, or that it did not throw, cannot see it —
27+
* that criterion IS the defect. Every assertion below reads the value in the
28+
* persisted `sys_job_run` / `sys_job` cell.
29+
*/
30+
31+
function silentLogger() {
32+
return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
33+
}
34+
35+
/** A fake job service for "process 1", which only has to park the run. */
36+
function fakeJobCtx() {
37+
const scheduled: Array<{ name: string; schedule: JobSchedule; handler: JobHandler }> = [];
38+
const cancelled: string[] = [];
39+
const job: IJobService = {
40+
async schedule(name, schedule, handler) { scheduled.push({ name, schedule, handler }); },
41+
async cancel(name) { cancelled.push(name); },
42+
async trigger() {},
43+
};
44+
const ctx = { logger: silentLogger(), getService: (id: string) => (id === 'job' ? job : undefined) } as any;
45+
return { ctx, scheduled, cancelled };
46+
}
47+
48+
function markerExecutor(ran: string[]): NodeExecutor {
49+
return { type: 'mark', async execute(node) { ran.push(node.id); return { success: true }; } };
50+
}
51+
52+
/** Minimal ObjectQL stand-in for the two audit tables `DbJobAdapter` writes. */
53+
function makeFakeEngine() {
54+
const tables = new Map<string, any[]>();
55+
return {
56+
tables,
57+
async find(table: string, opts: any = {}) {
58+
const t = tables.get(table) ?? [];
59+
const out = opts.where
60+
? t.filter((r) => Object.entries(opts.where).every(([k, v]) => r[k] === v))
61+
: [...t];
62+
return opts.limit ? out.slice(0, opts.limit) : out;
63+
},
64+
async insert(table: string, data: any) {
65+
const t = tables.get(table) ?? [];
66+
t.push({ ...data });
67+
tables.set(table, t);
68+
return { id: data.id };
69+
},
70+
async update(table: string, patch: any, options?: any) {
71+
// Same binding as the sibling double in `service-job`: the fake refuses
72+
// exactly what `ObjectQLEngine.update` refuses, so the audit writes this
73+
// test asserts are writes a real server would have accepted.
74+
assertEngineUpdateDispatch(patch, options);
75+
const t = tables.get(table) ?? [];
76+
const r = t.find((x) => x.id === patch.id);
77+
if (!r) throw new Error(`row ${patch.id} not in ${table}`);
78+
Object.assign(r, patch);
79+
return r;
80+
},
81+
};
82+
}
83+
84+
const waitFlow = (waitConfig: Record<string, unknown>) => ({
85+
name: 'wait_flow',
86+
label: 'Wait Flow',
87+
type: 'autolaunched',
88+
nodes: [
89+
{ id: 'start', type: 'start', label: 'Start' },
90+
{ id: 'pause', type: 'wait', label: 'Wait', waitEventConfig: waitConfig },
91+
{ id: 'after', type: 'mark', label: 'After' },
92+
{ id: 'end', type: 'end', label: 'End' },
93+
],
94+
edges: [
95+
{ id: 'e1', source: 'start', target: 'pause' },
96+
{ id: 'e2', source: 'pause', target: 'after' },
97+
{ id: 'e3', source: 'after', target: 'end' },
98+
],
99+
});
100+
101+
const config = { eventType: 'timer', timerDuration: 'P1D' };
102+
103+
/** A store whose resume-time `load` is unreachable; everything else works. */
104+
function storeWithUnreadableLoad(inner: InMemorySuspendedRunStore) {
105+
return {
106+
inner,
107+
async save(run: any) { return inner.save(run); },
108+
async load(_runId: string): Promise<any> { throw new Error('connection refused'); },
109+
async delete(runId: string) { return inner.delete(runId); },
110+
async list() { return inner.list(); },
111+
};
112+
}
113+
114+
/**
115+
* Park a run in "process 1", then cold-boot "process 2" whose durable read is
116+
* broken and whose job service is a real `DbJobAdapter`. Returns the adapter,
117+
* the fake ObjectQL tables, and the wake-up job's name.
118+
*/
119+
async function coldBootOntoDbJobAdapter(broken: boolean) {
120+
const inner = new InMemorySuspendedRunStore();
121+
const boot1 = fakeJobCtx();
122+
const e1 = new AutomationEngine(silentLogger());
123+
e1.registerNodeExecutor(markerExecutor([]));
124+
registerWaitNode(e1, boot1.ctx);
125+
e1.setSuspendedRunStore(inner);
126+
e1.registerFlow('wait_flow', waitFlow(config));
127+
const paused = await e1.execute('wait_flow');
128+
expect(paused.status).toBe('paused');
129+
130+
const store = broken ? (storeWithUnreadableLoad(inner) as any) : inner;
131+
const ran: string[] = [];
132+
const objectql = makeFakeEngine();
133+
const jobService = new DbJobAdapter({ engine: objectql });
134+
const e2 = new AutomationEngine(silentLogger());
135+
e2.registerNodeExecutor(markerExecutor(ran));
136+
// The wait node is registered against the REAL adapter, so the teardown that
137+
// fires when the run leaves the node (#5512) goes through it too.
138+
registerWaitNode(e2, {
139+
logger: silentLogger(),
140+
getService: (id: string) => (id === 'job' ? jobService : undefined),
141+
} as any);
142+
e2.setSuspendedRunStore(store);
143+
e2.registerFlow('wait_flow', waitFlow(config));
144+
// The re-arm pass registers the one-shot on the real adapter — from here on
145+
// every run of that job goes through `DbJobAdapter.wrap`.
146+
expect(await rearmSuspendedWaitTimers(e2, store, jobService, silentLogger())).toBe(1);
147+
148+
return { paused, inner, ran, objectql, jobService, jobName: `flow-wait:${paused.runId}:pause` };
149+
}
150+
151+
describe('#5548 — the #5529 wait wake-up that consumed nothing is audited as degraded, not success', () => {
152+
it('the wake-up into an unreachable store lands sys_job_run.status = "degraded"', async () => {
153+
const { inner, ran, objectql, jobService, jobName, paused } = await coldBootOntoDbJobAdapter(true);
154+
155+
// Fire the wake-up the way an operator or the timer would.
156+
await jobService.trigger(jobName);
157+
158+
// The pause really was not consumed — the run is still parked, its row still
159+
// there. This is the premise the audit row has to reflect.
160+
expect(ran).toEqual([]);
161+
expect((await inner.list()).map((r) => r.runId)).toEqual([paused.runId]);
162+
163+
const runs = objectql.tables.get('sys_job_run') ?? [];
164+
expect(runs).toHaveLength(1);
165+
expect(runs[0].job_name).toBe(jobName);
166+
// The cell this card exists for. Before the wiring it read 'success'.
167+
expect(runs[0].status).toBe('degraded');
168+
expect(runs[0].error).toBe('STORE_UNAVAILABLE');
169+
170+
await jobService.destroy();
171+
});
172+
173+
it('the job row mirrors it, stays active, and does NOT count as a failure', async () => {
174+
const { objectql, jobService, jobName } = await coldBootOntoDbJobAdapter(true);
175+
await jobService.trigger(jobName);
176+
177+
const [job] = objectql.tables.get('sys_job') ?? [];
178+
expect(job.last_status).toBe('degraded');
179+
expect(job.last_error).toBe('STORE_UNAVAILABLE');
180+
// #5529's half is untouched: the one-shot is kept ARMED so the stuck run
181+
// stays visible and the wake-up re-firable.
182+
expect(job.active).toBe(true);
183+
// `degraded` is not a failure: the retry/alerting signal does not move.
184+
expect(job.failure_count).toBe(0);
185+
expect(job.run_count).toBe(1);
186+
187+
await jobService.destroy();
188+
});
189+
190+
it('a wake-up that DOES resume the run is still audited as success (control)', async () => {
191+
const { ran, objectql, jobService, jobName } = await coldBootOntoDbJobAdapter(false);
192+
await jobService.trigger(jobName);
193+
194+
// The pause was consumed and traversal continued…
195+
expect(ran).toEqual(['after']);
196+
const runs = objectql.tables.get('sys_job_run') ?? [];
197+
expect(runs).toHaveLength(1);
198+
// …so the row says success, exactly as before this change.
199+
expect(runs[0].status).toBe('success');
200+
expect(runs[0].error).toBeNull();
201+
const [job] = objectql.tables.get('sys_job') ?? [];
202+
expect(job.last_status).toBe('success');
203+
// The one-shot had its shot and settled the pause, so it disarms — the
204+
// `sys_job` row goes inactive, which is the OPPOSITE of the degraded case.
205+
expect(job.active).toBe(false);
206+
207+
await jobService.destroy();
208+
});
209+
});

packages/services/service-automation/src/builtin/wait-node.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import type { PluginContext } from '@objectstack/core';
44
import { defineActionDescriptor } from '@objectstack/spec/automation';
5-
import type { IJobService } from '@objectstack/spec/contracts';
5+
import type { IJobService, JobRunOutcome } from '@objectstack/spec/contracts';
66
import type { AutomationEngine, SuspendedRunStore } from '../engine.js';
77
import { describeThrownForLog, type ThrownCauseMeta } from '../thrown-cause-diagnostics.js';
88

@@ -56,6 +56,13 @@ interface WaitTimerLogger {
5656
* healthy timer is the durability degradation AGENTS.md's log-level rule is
5757
* about (#4632), and this path was previously silent — the result was
5858
* discarded by the callback without so much as a `warn`.
59+
*
60+
* Since #5548 this outcome is also RESOLVED as `{ outcome: 'degraded',
61+
* reason: 'STORE_UNAVAILABLE' }`, so the job service records the run as
62+
* `degraded` rather than `success` — the log line said the shot missed, the
63+
* audit row said it succeeded. The `reason` is the short code only: the
64+
* driver's own message can be multi-line and belongs in the log record's
65+
* `meta` (#5737), not in an audit column.
5966
* - **everything else** — cancel, exactly as before. Success consumed the
6067
* pause; `RESUME_IN_PROGRESS` means a concurrent resume is consuming it (and
6168
* #5512's `onSuspensionReleased` drops this job when it does); a machine-state
@@ -89,15 +96,26 @@ function makeWaitTimerJobHandler(
8996
runId: string,
9097
jobName: string,
9198
logger: WaitTimerLogger,
92-
): () => Promise<void> {
99+
): () => Promise<void | JobRunOutcome> {
93100
return async () => {
94101
// Set only on the one outcome that must NOT disarm the job. A thrown
95102
// `resume` leaves it false, so the `finally` still cancels.
96103
let keepArmed = false;
104+
// #5548 — what this shot reports to the JOB service, as opposed to what it
105+
// logs. `STORE_UNAVAILABLE` is the specimen the ruling names: the handler
106+
// completes normally (deliberately — #5529 refused to make it throw,
107+
// because a throw is the retry signal third-party `IJobService`
108+
// implementations key on), so before the `JobRunOutcome` channel existed
109+
// the run was recorded as `success` on an audit surface whose whole job is
110+
// to say whether the work happened. Resolving `degraded` instead moves the
111+
// `sys_job_run` row and nothing else: still no throw, still no retry, and
112+
// the job still stays ARMED and `active` exactly as #5529 fixed it.
113+
let outcome: JobRunOutcome | undefined;
97114
try {
98115
const result = await engine.resume(runId);
99116
if (result?.code === 'STORE_UNAVAILABLE') {
100117
keepArmed = true;
118+
outcome = { outcome: 'degraded', reason: 'STORE_UNAVAILABLE' };
101119
// #5737 — the cause goes to `meta`, never into the message. This one is
102120
// NOT a thrown value and so does NOT go through `describeThrownForLog`:
103121
// `AutomationResult.error` is a STRING the engine already composed
@@ -138,6 +156,9 @@ function makeWaitTimerJobHandler(
138156
}
139157
}
140158
}
159+
// Resolved, never thrown — the report rides the RETURN value precisely so
160+
// the failure semantics of `IJobService` stay untouched (#6617).
161+
return outcome;
141162
};
142163
}
143164

packages/services/service-job/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"croner": "^10.0.1"
2626
},
2727
"devDependencies": {
28+
"@objectstack/metadata-core": "workspace:*",
2829
"@types/node": "^26.1.2",
2930
"typescript": "^6.0.3",
3031
"vitest": "^4.1.10"

packages/services/service-job/src/cron-job-adapter.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,16 @@ export class CronJobAdapter implements IJobService {
155155
};
156156
const startMs = Date.now();
157157
try {
158-
await runWithPolicy(record.name, () => record.handler({ jobId: record.name, data }), record.options);
159-
execution.status = 'success';
158+
const outcome = await runWithPolicy(record.name, () => record.handler({ jobId: record.name, data }), record.options);
159+
// #5548 — same mapping as `IntervalJobAdapter.executeJob`, deliberately
160+
// one shape and not two spellings: a resolved `degraded` outcome is a
161+
// completed run whose work did not happen, never a `success`.
162+
if (outcome && outcome.outcome === 'degraded') {
163+
execution.status = 'degraded';
164+
execution.error = outcome.reason;
165+
} else {
166+
execution.status = 'success';
167+
}
160168
} catch (err) {
161169
execution.status = err instanceof JobTimeoutError ? 'timeout' : 'failed';
162170
execution.error = err instanceof Error ? err.message : String(err);

0 commit comments

Comments
 (0)