Skip to content

Commit 0085131

Browse files
Pijukatelclaude
andauthored
Fix deleting Actors and log timestamps (#30)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 86988d3 commit 0085131

6 files changed

Lines changed: 197 additions & 17 deletions

File tree

requirements/api.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434
- `GET /actor-runtime/events/:runId`: a websocket upgrade, not a JSON response at all - see "Actor
3535
runtime API" below.
3636
- `*At` timestamp fields are ISO-8601 strings.
37+
- Log content matches the Apify platform's log format: every log line starts with an ISO-8601 UTC
38+
timestamp with millisecond precision followed by a space (`2026-08-31T09:13:25.123Z `), exactly one
39+
timestamp per line regardless of how the output was chunked when produced. Apify clients' log
40+
redirection (e.g. `Actor.call` in the SDKs) relies on this prefix to recognize log messages.
3741

3842
# Actor id encoding
3943

src/api/routes/actors.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ export function mountActors(router: Router, deps: ApiServerDeps): void {
9494
req.params.actorId as string,
9595
requireUser(req).username,
9696
);
97-
if (actor) await deleteActor(actor.id);
97+
// Matches the real platform: DELETE of a missing Actor 404s the same as GET (api.md's
98+
// "applies uniformly to every DELETE").
99+
if (!actor) throw recordNotFound();
100+
await deleteActor(actor.id);
98101
res.status(204).end();
99102
}),
100103
);
@@ -182,12 +185,13 @@ export function mountActors(router: Router, deps: ApiServerDeps): void {
182185
req.params.actorId as string,
183186
requireUser(req).username,
184187
);
185-
if (actor) {
186-
await updateActor(actor.id, (current) => ({
187-
...current,
188-
versions: current.versions.filter((v) => v.versionNumber !== req.params.versionNumber),
189-
}));
190-
}
188+
// Matches the real platform: a missing Actor and a missing version both 404, never a silent 204.
189+
if (!actor) throw recordNotFound();
190+
if (!findVersion(actor, req.params.versionNumber as string)) throw recordNotFound();
191+
await updateActor(actor.id, (current) => ({
192+
...current,
193+
versions: current.versions.filter((v) => v.versionNumber !== req.params.versionNumber),
194+
}));
191195
res.status(204).end();
192196
}),
193197
);

src/services/logs.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ interface LiveLog {
99
buffer: string[];
1010
subscribers: Set<(chunk: string) => void>;
1111
terminal: boolean;
12+
/** Whether the next appended character starts a new log line - lets `appendLog` stamp exactly one
13+
* timestamp per line even when chunk boundaries fall mid-line. */
14+
atLineStart: boolean;
1215
}
1316

1417
const live = new Map<string, LiveLog>();
@@ -26,17 +29,41 @@ const flushMutex = new KeyedMutex();
2629
function getOrCreate(id: string): LiveLog {
2730
let state = live.get(id);
2831
if (!state) {
29-
state = { buffer: [], subscribers: new Set(), terminal: false };
32+
state = { buffer: [], subscribers: new Set(), terminal: false, atLineStart: true };
3033
live.set(id, state);
3134
}
3235
return state;
3336
}
3437

38+
/**
39+
* Prefixes every log *line* in `chunk` with an ingestion timestamp (`2026-08-31T09:13:25.123Z `), the
40+
* platform's log format (api.md). Apify clients' log redirection recognizes messages by this prefix,
41+
* so unstamped lines would never be redirected.
42+
*/
43+
function stampLines(state: LiveLog, chunk: string): string {
44+
let out = '';
45+
let from = 0;
46+
while (from < chunk.length) {
47+
if (state.atLineStart) out += `${new Date().toISOString()} `;
48+
const newlineAt = chunk.indexOf('\n', from);
49+
if (newlineAt === -1) {
50+
out += chunk.slice(from);
51+
state.atLineStart = false;
52+
break;
53+
}
54+
out += chunk.slice(from, newlineAt + 1);
55+
state.atLineStart = true;
56+
from = newlineAt + 1;
57+
}
58+
return out;
59+
}
60+
3561
export function appendLog(id: string, chunk: string): void {
3662
if (!chunk) return;
3763
const state = getOrCreate(id);
38-
state.buffer.push(chunk);
39-
for (const subscriber of state.subscribers) subscriber(chunk);
64+
const stamped = stampLines(state, chunk);
65+
state.buffer.push(stamped);
66+
for (const subscriber of state.subscribers) subscriber(stamped);
4067
}
4168

4269
/** Returns an unsubscribe function. */

test/integration/actors-builds-runs.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,59 @@ describe('actors / versions / builds / runs (via real apify-client)', () => {
310310
expect(body.error.type).toBe('record-not-found');
311311
});
312312

313+
it('deletes an Actor for real, and a missing Actor 404s on DELETE the same as on GET (record-not-found)', async () => {
314+
const actor = await server.client.actors().create({ name: 'delete-actor' });
315+
const user = await server.client.user('me').get();
316+
317+
await server.client.actor(actor.id).delete();
318+
expect(await server.client.actor(actor.id).get()).toBeUndefined();
319+
320+
const list = await server.client.actors().list();
321+
expect(list.items.some((a) => a.id === actor.id)).toBe(false);
322+
323+
// apify-client-js's own `.delete()` swallows a `record-not-found` 404 to stay idempotent from the
324+
// caller's perspective, so hit the HTTP endpoint directly to observe the real status/envelope.
325+
for (const missingRef of ['nonexistent12345b', `${user.username}~does-not-exist`]) {
326+
const res = await fetch(`${server.baseUrl}/v2/actors/${missingRef}`, {
327+
method: 'DELETE',
328+
headers: { Authorization: `Bearer ${server.token}` },
329+
});
330+
expect(res.status).toBe(404);
331+
const body = (await res.json()) as { error: { type: string } };
332+
expect(body.error.type).toBe('record-not-found');
333+
}
334+
});
335+
336+
it('deletes an Actor version for real, and a missing version or Actor 404s on DELETE (record-not-found)', async () => {
337+
const actor = await server.client.actors().create({ name: 'delete-version-actor' });
338+
await server.client
339+
.actor(actor.id)
340+
.versions()
341+
.create({
342+
versionNumber: '0.0',
343+
buildTag: 'latest',
344+
sourceType: 'SOURCE_FILES' as never,
345+
sourceFiles: [],
346+
} as never);
347+
348+
await server.client.actor(actor.id).version('0.0').delete();
349+
expect(await server.client.actor(actor.id).version('0.0').get()).toBeUndefined();
350+
351+
const missingVersion = await fetch(`${server.baseUrl}/v2/actors/${actor.id}/versions/9.9`, {
352+
method: 'DELETE',
353+
headers: { Authorization: `Bearer ${server.token}` },
354+
});
355+
expect(missingVersion.status).toBe(404);
356+
expect(((await missingVersion.json()) as { error: { type: string } }).error.type).toBe('record-not-found');
357+
358+
const missingActor = await fetch(`${server.baseUrl}/v2/actors/nonexistent12345c/versions/0.0`, {
359+
method: 'DELETE',
360+
headers: { Authorization: `Bearer ${server.token}` },
361+
});
362+
expect(missingActor.status).toBe(404);
363+
expect(((await missingActor.json()) as { error: { type: string } }).error.type).toBe('record-not-found');
364+
});
365+
313366
it('DELETE rejects a non-terminal build instead of deleting it (matches the real platform: reject, not abort-then-delete)', async () => {
314367
const actor = await server.client.actors().create({ name: 'delete-running-build-actor' });
315368
const { builds } = getRegistries();

test/integration/log-drain-race.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ describe('end-to-end: a run never reads terminal via HTTP before its log has ful
147147
// A fresh, non-stream log read the instant status is observed terminal must contain the full
148148
// output - not just what had arrived before the container "exited".
149149
const log = await server.client.log(run.id).get();
150-
expect(log).toBe('final line\n');
150+
// Strip the per-line timestamp - this test is about the drain race, not the log format.
151+
expect(log?.replace(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z /, '')).toBe('final line\n');
151152
});
152153
});

test/integration/logs.test.ts

Lines changed: 97 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,16 @@ import {
1212
import { getRegistries } from '../../src/storage/registries.js';
1313
import { getOrCreateUserForToken } from '../../src/services/users.js';
1414

15+
/** The per-line timestamp prefix from the platform log format (api.md). */
16+
const LINE_STAMP = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z /;
17+
18+
function stripStamps(log: string): string {
19+
return log
20+
.split('\n')
21+
.map((line) => line.replace(LINE_STAMP, ''))
22+
.join('\n');
23+
}
24+
1525
/** Polls `check()` until it returns true or `timeoutMs` elapses, rather than a fixed sleep - keeps the
1626
* disconnect test fast on the happy path and still deterministic if cleanup is ever slow. */
1727
async function waitUntil(check: () => boolean, timeoutMs = 2000): Promise<void> {
@@ -195,7 +205,7 @@ describe('log streaming', () => {
195205
await second;
196206
spy.mockRestore();
197207

198-
expect(await getFullLog(jobId)).toBe('line1\nline2\n');
208+
expect(stripStamps(await getFullLog(jobId))).toBe('line1\nline2\n');
199209
});
200210

201211
it('getFullLog never returns a torn read while a flush for the same id is in flight (regression for the reader-vs-writer race)', async () => {
@@ -232,7 +242,7 @@ describe('log streaming', () => {
232242
const result = await read;
233243
spy.mockRestore();
234244

235-
expect(result).toBe('first chunk\n');
245+
expect(stripStamps(result)).toBe('first chunk\n');
236246
});
237247

238248
it("flushAllLogs persists every live log's buffered content (regression for a lost trailing chunk on graceful shutdown)", async () => {
@@ -248,8 +258,8 @@ describe('log streaming', () => {
248258

249259
await flushAllLogs();
250260

251-
expect(await getRegistries().logs.getValue(jobId1)).toBe('alpha\n');
252-
expect(await getRegistries().logs.getValue(jobId2)).toBe('beta\n');
261+
expect(stripStamps((await getRegistries().logs.getValue(jobId1)) ?? '')).toBe('alpha\n');
262+
expect(stripStamps((await getRegistries().logs.getValue(jobId2)) ?? '')).toBe('beta\n');
253263
});
254264

255265
it('a stream closes when the persisted record turns terminal even if markLogTerminal is never called (regression: aborted in the READY window)', async () => {
@@ -401,7 +411,7 @@ describe('log streaming', () => {
401411

402412
const reader = res.body!.getReader();
403413
const { value } = await reader.read();
404-
expect(Buffer.from(value!).toString()).toBe('line 1\n');
414+
expect(stripStamps(Buffer.from(value!).toString())).toBe('line 1\n');
405415

406416
// The initial response is a single `res.status(200).send(soFar)`, never `res.write` + a
407417
// held-open connection, so `subscribeLog` is never called - checked immediately after the first
@@ -444,10 +454,91 @@ describe('log streaming', () => {
444454

445455
const reader = res.body!.getReader();
446456
const { value } = await reader.read();
447-
expect(Buffer.from(value!).toString()).toBe('line 1\n');
457+
expect(stripStamps(Buffer.from(value!).toString())).toBe('line 1\n');
448458
expect(getSubscriberCount(jobId)).toBe(0);
449459

450460
const { done } = await reader.read();
451461
expect(done).toBe(true);
452462
});
463+
464+
it('stamps every log line with a platform-style ISO timestamp, exactly once per line even when chunk boundaries fall mid-line', async () => {
465+
const jobId = 'stampedLinesJobId12';
466+
// Docker output is not line-aligned: one line arrives split over two appends, and one append
467+
// carries two lines. Both must come out with exactly one stamp per *line*.
468+
appendLog(jobId, '[apify] INFO first line\n[apify] WARN second li');
469+
appendLog(jobId, 'ne, same stamp\n');
470+
appendLog(jobId, '[apify] INFO third line\n');
471+
472+
const log = await getFullLog(jobId);
473+
const lines = log.split('\n').filter((line) => line.length > 0);
474+
expect(lines).toHaveLength(3);
475+
for (const line of lines) expect(line).toMatch(LINE_STAMP);
476+
// The continuation of the split line must NOT have been stamped mid-line.
477+
expect(lines[1]).toMatch(/second line, same stamp$/);
478+
expect(stripStamps(log)).toBe(
479+
'[apify] INFO first line\n[apify] WARN second line, same stamp\n[apify] INFO third line\n',
480+
);
481+
});
482+
483+
it('apify-client log redirection recovers every message from ?stream=true&raw=true (regression: Actor.call redirected nothing)', async () => {
484+
const jobId = 'redirectedLogJobId1';
485+
appendLog(jobId, '[apify] INFO Initializing Actor...\n');
486+
487+
const user = await getOrCreateUserForToken(server.token);
488+
await getRegistries().runs.set(jobId, {
489+
id: jobId,
490+
userId: user.id,
491+
actorId: 'x',
492+
buildId: 'y',
493+
buildNumber: '0.0.1',
494+
status: 'RUNNING',
495+
startedAt: new Date().toISOString(),
496+
defaultDatasetId: 'd',
497+
defaultKeyValueStoreId: 'k',
498+
defaultRequestQueueId: 'r',
499+
options: { memoryMbytes: 1024, timeoutSecs: 300 },
500+
meta: { origin: 'API' },
501+
});
502+
503+
// The exact request apify-client's log redirection makes.
504+
const res = await fetch(`${server.baseUrl}/v2/actor-runs/${jobId}/log?stream=true&raw=true`, {
505+
headers: { Authorization: `Bearer ${server.token}` },
506+
});
507+
expect(res.status).toBe(200);
508+
509+
setTimeout(() => appendLog(jobId, '[apify] INFO doing work\n[apify] WARN partial li'), 50);
510+
setTimeout(() => appendLog(jobId, 'ne finished\n'), 100);
511+
setTimeout(() => markLogTerminal(jobId), 150);
512+
513+
// Replicates the client's redirect parsing: buffer chunks, split on the timestamp marker, emit
514+
// only marker-delimited messages (a possibly incomplete trailing part waits in the buffer until
515+
// the final flush). Without the per-line timestamps this recovers zero messages.
516+
const splitMarker = /(?:\n|^)(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z)/;
517+
let streamBuffer = '';
518+
const messages: string[] = [];
519+
const flushBuffer = (includeLastPart: boolean) => {
520+
const allParts = streamBuffer.split(splitMarker).slice(1);
521+
const complete = includeLastPart ? allParts : allParts.slice(0, -2);
522+
streamBuffer = includeLastPart ? '' : allParts.slice(-2).join('');
523+
for (let i = 0; i + 1 < complete.length; i += 2) {
524+
messages.push(`${complete[i]}${complete[i + 1]}`.trim());
525+
}
526+
};
527+
528+
const reader = res.body!.getReader();
529+
for (;;) {
530+
const { value, done } = await reader.read();
531+
if (done) break;
532+
streamBuffer += Buffer.from(value!).toString();
533+
if (splitMarker.test(streamBuffer)) flushBuffer(false);
534+
}
535+
flushBuffer(true);
536+
537+
const contentsOnly = messages.map((message) => message.replace(LINE_STAMP, ''));
538+
expect(contentsOnly).toEqual([
539+
'[apify] INFO Initializing Actor...',
540+
'[apify] INFO doing work',
541+
'[apify] WARN partial line finished',
542+
]);
543+
});
453544
});

0 commit comments

Comments
 (0)