Skip to content

Commit c6da7ef

Browse files
PaulHaxPaulHax
committed
fix(processing): refetch a job's details once it finishes
Opening a running job's details cached the log as it was at that moment, so a job that then failed kept showing a traceback cut off before the error line until VolView was reloaded. Record whether each detail request was issued before the job finished. When the provider reports the job terminal, drop a detail fetched earlier and fetch it again. Only the latest request's response is kept, so a slow earlier response cannot replace the final log. Rows that were never opened and synthesized failures fetch nothing.
1 parent ce1ad67 commit c6da7ef

5 files changed

Lines changed: 304 additions & 39 deletions

File tree

‎src/processing/__tests__/fakeProvider.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { vi } from 'vitest';
22
import type {
3+
ProcessingJobStatus,
34
ProcessingProvider,
45
ProcessingProviderConfig,
56
} from '@/src/processing/types';
@@ -33,3 +34,38 @@ export const makeFakeProvider = (
3334
getJobHistoryDetail: vi.fn(),
3435
...overrides,
3536
}) as FakeProvider;
37+
38+
export const resultStateFor = (state: ProcessingJobStatus['state']) =>
39+
state === 'success'
40+
? ('ready' as const)
41+
: state === 'error' || state === 'cancelled'
42+
? ('unavailable' as const)
43+
: ('waiting' as const);
44+
45+
export const jobStatus = (
46+
jobId: string,
47+
state: ProcessingJobStatus['state'],
48+
extra: Partial<ProcessingJobStatus> = {}
49+
): ProcessingJobStatus => ({
50+
jobId,
51+
state,
52+
resultState: resultStateFor(state),
53+
...extra,
54+
});
55+
56+
type ProviderRegistry = {
57+
registerProviderConfig(config: ProcessingProviderConfig): void;
58+
instances: Map<string, ProcessingProvider>;
59+
};
60+
61+
// Seating the instance is what a transport load would have produced.
62+
export const registerFake = (
63+
store: ProviderRegistry,
64+
provider: FakeProvider
65+
) => {
66+
store.registerProviderConfig(provider.config);
67+
store.instances.set(
68+
provider.config.id,
69+
provider as unknown as ProcessingProvider
70+
);
71+
};

‎src/processing/__tests__/store.spec.ts‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
} from '@/src/processing/types';
1919
import { jobKey } from '@/src/processing/types';
2020
import { defer } from '@/src/utils';
21-
import { makeFakeProvider } from './fakeProvider';
21+
import { jobStatus, makeFakeProvider, resultStateFor } from './fakeProvider';
2222
import {
2323
seatDataSource,
2424
seatVolume,
@@ -67,24 +67,6 @@ const sampleResults: ProcessingResult[] = [
6767
{ id: 'r1', name: 'out.nrrd', url: 'http://localhost/out.nrrd' },
6868
];
6969

70-
const resultStateFor = (state: ProcessingJobStatus['state']) =>
71-
state === 'success'
72-
? ('ready' as const)
73-
: state === 'error' || state === 'cancelled'
74-
? ('unavailable' as const)
75-
: ('waiting' as const);
76-
77-
const jobStatus = (
78-
jobId: string,
79-
state: ProcessingJobStatus['state'],
80-
extra: Partial<ProcessingJobStatus> = {}
81-
): ProcessingJobStatus => ({
82-
jobId,
83-
state,
84-
resultState: resultStateFor(state),
85-
...extra,
86-
});
87-
8870
const resultsBundle = (results: ProcessingResult[] = [], missing = 0) => ({
8971
results,
9072
missing,

‎src/processing/components/__tests__/JobList.spec.ts‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest';
2+
import { shallowMount, type VueWrapper } from '@vue/test-utils';
3+
import { createPinia, setActivePinia } from 'pinia';
4+
import { createApp } from 'vue';
5+
6+
import type { JobHistoryDetail } from '@/backend-contract';
7+
import { CorePiniaProviderPlugin } from '@/src/core/provider';
8+
import { defer } from '@/src/utils';
9+
import type {
10+
ProcessingJobStatus,
11+
ProcessingProvider,
12+
} from '@/src/processing/types';
13+
import {
14+
jobStatus,
15+
makeFakeProvider,
16+
registerFake,
17+
type FakeProvider,
18+
} from '@/src/processing/__tests__/fakeProvider';
19+
import JobList from '@/src/processing/components/JobList.vue';
20+
import {
21+
POLL_INTERVAL_MS,
22+
useProcessingJobsStore,
23+
} from '@/src/processing/store';
24+
25+
const JOB_ID = 'job-1';
26+
const PARTIAL_LOG = [
27+
'Traceback (most recent call last):\n',
28+
' File "/opt/cli/volview_cli_base/cli.py", line 25, in run\n',
29+
];
30+
const FINAL_LINE = 'ValueError: Lower threshold must be <= upper threshold';
31+
const FULL_LOG = [...PARTIAL_LOG, `${FINAL_LINE}\n`];
32+
33+
const detail = (log: string[]): JobHistoryDetail => ({
34+
jobId: JOB_ID,
35+
log,
36+
parameters: {},
37+
});
38+
39+
const failed = jobStatus(JOB_ID, 'error', {
40+
errorTail: 'DockerException: exit code 1',
41+
});
42+
43+
const makeProvider = (
44+
overrides: Partial<ProcessingProvider> = {}
45+
): FakeProvider =>
46+
makeFakeProvider(
47+
{
48+
id: 'p1',
49+
label: 'P1',
50+
baseUrl: 'http://p1/',
51+
jobsBaseUrl: 'http://p1/jobs',
52+
},
53+
{
54+
runTask: vi.fn().mockResolvedValue({ jobId: JOB_ID }),
55+
getJob: vi
56+
.fn()
57+
.mockResolvedValueOnce(jobStatus(JOB_ID, 'running'))
58+
.mockResolvedValue(failed),
59+
...overrides,
60+
}
61+
);
62+
63+
// Clicks reach toggleDetails through the attribute fallthrough on <button>.
64+
const btnStub = { template: '<button><slot /></button>' };
65+
const slotStub = { template: '<div><slot /></div>' };
66+
67+
describe('JobList error log', () => {
68+
let pinia: ReturnType<typeof createPinia>;
69+
let wrapper: VueWrapper | undefined;
70+
71+
beforeEach(() => {
72+
vi.useFakeTimers();
73+
pinia = createPinia().use(CorePiniaProviderPlugin());
74+
createApp({}).use(pinia);
75+
setActivePinia(pinia);
76+
});
77+
78+
afterEach(() => {
79+
wrapper?.unmount();
80+
wrapper = undefined;
81+
vi.useRealTimers();
82+
});
83+
84+
// A bound base image keeps the completion path from fetching details to
85+
// rebuild an adopted job's parent, so every detail request comes from the row.
86+
const submit = async (provider: FakeProvider) => {
87+
const store = useProcessingJobsStore();
88+
registerFake(store, provider);
89+
await store.submitJob(
90+
provider.config.id,
91+
'threshold',
92+
{},
93+
{
94+
activeDatasetId: 'base',
95+
}
96+
);
97+
await vi.advanceTimersByTimeAsync(0);
98+
};
99+
100+
const mountList = () => {
101+
wrapper = shallowMount(JobList, {
102+
global: {
103+
plugins: [pinia],
104+
stubs: {
105+
'v-btn': btnStub,
106+
'expand-transition': slotStub,
107+
VSelect: { template: '<div />' },
108+
},
109+
},
110+
});
111+
return wrapper;
112+
};
113+
114+
const expandDetails = async (list: VueWrapper) => {
115+
await list.find('.details-btn').trigger('click');
116+
await vi.advanceTimersByTimeAsync(0);
117+
};
118+
119+
it('shows the final log when a job fails after its details were opened', async () => {
120+
const provider = makeProvider({
121+
getJobHistoryDetail: vi
122+
.fn()
123+
.mockResolvedValueOnce(detail(PARTIAL_LOG))
124+
.mockResolvedValue(detail(FULL_LOG)),
125+
});
126+
await submit(provider);
127+
const list = mountList();
128+
129+
await expandDetails(list);
130+
await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS);
131+
132+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
133+
expect(provider.getJobHistoryDetail).toHaveBeenCalledTimes(2);
134+
});
135+
136+
it('keeps the final log when the earlier detail response arrives last', async () => {
137+
const partial = defer<JobHistoryDetail>();
138+
const provider = makeProvider({
139+
getJobHistoryDetail: vi
140+
.fn()
141+
.mockReturnValueOnce(partial.promise)
142+
.mockResolvedValue(detail(FULL_LOG)),
143+
});
144+
await submit(provider);
145+
const list = mountList();
146+
147+
await expandDetails(list);
148+
await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS);
149+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
150+
151+
partial.resolve(detail(PARTIAL_LOG));
152+
await vi.advanceTimersByTimeAsync(0);
153+
154+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
155+
});
156+
157+
// Adoption completes a job it read once, without registering a poller.
158+
it('keeps the final log when an adopted job fails while its earlier detail is in flight', async () => {
159+
const partial = defer<JobHistoryDetail>();
160+
const adoptedStatus = defer<ProcessingJobStatus>();
161+
const provider = makeProvider({
162+
listJobHistory: vi.fn().mockResolvedValue({
163+
jobs: [
164+
{
165+
jobId: JOB_ID,
166+
taskId: 'threshold',
167+
taskTitle: 'Threshold',
168+
createdBy: { id: 'u1', name: 'User One' },
169+
createdAt: '2026-09-21T12:00:00Z',
170+
state: 'running',
171+
resultState: 'waiting',
172+
},
173+
],
174+
nextCursor: null,
175+
}),
176+
getJob: vi.fn().mockReturnValue(adoptedStatus.promise),
177+
getJobHistoryDetail: vi
178+
.fn()
179+
.mockReturnValueOnce(partial.promise)
180+
.mockResolvedValue(detail(FULL_LOG)),
181+
});
182+
const store = useProcessingJobsStore();
183+
registerFake(store, provider);
184+
const adoption = store.adoptJobHistory();
185+
await vi.advanceTimersByTimeAsync(0);
186+
const list = mountList();
187+
188+
await expandDetails(list);
189+
adoptedStatus.resolve(failed);
190+
await adoption;
191+
await vi.advanceTimersByTimeAsync(0);
192+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
193+
194+
partial.resolve(detail(PARTIAL_LOG));
195+
await vi.advanceTimersByTimeAsync(0);
196+
197+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
198+
});
199+
200+
it('fetches the details of an already failed job once', async () => {
201+
const provider = makeProvider({
202+
runTask: vi.fn().mockResolvedValue({ jobId: JOB_ID, status: failed }),
203+
getJobHistoryDetail: vi.fn().mockResolvedValue(detail(FULL_LOG)),
204+
});
205+
await submit(provider);
206+
const list = mountList();
207+
208+
await expandDetails(list);
209+
await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS * 5);
210+
211+
expect(list.find('.error-log').text()).toContain(FINAL_LINE);
212+
expect(provider.getJobHistoryDetail).toHaveBeenCalledTimes(1);
213+
});
214+
215+
it('fetches no details when a job whose details were never opened finishes', async () => {
216+
const provider = makeProvider({
217+
getJobHistoryDetail: vi.fn().mockResolvedValue(detail(FULL_LOG)),
218+
});
219+
await submit(provider);
220+
const list = mountList();
221+
222+
await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS * 5);
223+
224+
expect(list.find('.job-row').exists()).toBe(true);
225+
expect(provider.getJobHistoryDetail).not.toHaveBeenCalled();
226+
});
227+
});

‎src/processing/components/__tests__/JobsModule.spec.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import { createApp } from 'vue';
66
import { CorePiniaProviderPlugin } from '@/src/core/provider';
77
import { defer, type Deferred } from '@/src/utils';
88
import type {
9-
ProcessingProvider,
109
ProcessingProviderConfig,
1110
TaskSummary,
1211
} from '@/src/processing/types';
1312
import type { TaskSpecEnvelope } from '@/src/processing/engine/taskSpec';
1413
import {
1514
makeFakeProvider,
15+
registerFake,
1616
type FakeProvider,
1717
} from '@/src/processing/__tests__/fakeProvider';
1818

@@ -65,18 +65,6 @@ const envelope = (id: string, title: string): TaskSpecEnvelope => ({
6565
outputs: [],
6666
});
6767

68-
const registerFake = (
69-
store: ReturnType<typeof useProcessingJobsStore>,
70-
provider: FakeProvider
71-
) => {
72-
store.registerProviderConfig(provider.config);
73-
// Seating the instance is what a transport load would have produced.
74-
store.instances.set(
75-
provider.config.id,
76-
provider as unknown as ProcessingProvider
77-
);
78-
};
79-
8068
// Auto-stubs drop slot content, hiding the panel children.
8169
const slotStub = { template: '<div><slot /></div>' };
8270

0 commit comments

Comments
 (0)