Skip to content

feat(push): add final status block and structured JSON output #1199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
l2ysho wants to merge 4 commits into
base: master
Choose a base branch
from
+133 −33
Open

feat(push): add final status block and structured JSON output #1199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1122ce2
feat(push): add final status block and structured JSON output
l2ysho Jun 15, 2026
6322f2c
add unit test
l2ysho Jun 15, 2026
e3cb16f
Merge branch 'master' into 1136-improve-apify-push-final-status-and-j…
l2ysho Jun 22, 2026
b55a4e3
Merge branch 'master' into 1136-improve-apify-push-final-status-and-j…
l2ysho Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 105 additions & 33 deletions src/commands/actors/push.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { CommandExitCodes, DEPRECATED_LOCAL_CONFIG_NAME, LOCAL_CONFIG_PATH } fro
import { sumFilesSizeInBytes } from '../../lib/files.js';
import { useAbortJobOnSignal } from '../../lib/hooks/useAbortJobOnSignal.js';
import { useActorConfig } from '../../lib/hooks/useActorConfig.js';
import { error, info, link, run, success, warning } from '../../lib/outputs.js';
import { error, info, run, simpleLog, warning } from '../../lib/outputs.js';
import { transformEnvToEnvVars } from '../../lib/secrets.js';
import {
createActZip,
Expand All @@ -48,6 +48,54 @@ const DEFAULT_ACTOR_VERSION_NUMBER = '0.0';
// that changes, we have to add it.
const DEFAULT_BUILD_TAG = 'latest';

// How many trailing log lines to surface as the failure reason.
const BUILD_LOG_TAIL_LINES = 10;

interface PushResult {

@DaveHanns DaveHanns Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prefer type over interface.

See https://www.notion.so/apify/Apify-Coding-Standards-Guide-264f39950a2280588be3f2268b9cb609?source=copy_link#264f39950a2280898b98c84f9342d686

ok: boolean;
operation: 'push';
actor: { id: string; url: string };
build: { id: string; number: string; status: string; url: string };
error?: { phase: 'build'; message: string; logTail: string[] };
exitCode: number;
}

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: to avoid type-checking for optional error, we could leverage discriminated union here:

type PushResultBase = {
	operation: 'push';
	actor: { id: string; url: string };
	build: { id: string; number: string; status: string; url: string };
	exitCode: number;
};
type PushSuccess = PushResultBase & {
	ok: true;
};
type PushFailure = PushResultBase & {
	ok: false;
	error: { phase: 'build'; message: string; logTail: string[] };
};
type PushResult = PushSuccess | PushFailure;


// Maps the final build status to the overall push outcome. `ok` mirrors command
// success (true iff exitCode is 0): a still-running fire-and-forget build is not
// a failure — its pending state is conveyed by the build status, not by `ok`.
export function resolvePushOutcome(buildStatus: string): {
resultLabel: string;
exitCode: number;
ok: boolean;
errorMessage?: string;
} {

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the return type would deserve to be extracted into its own type.

switch (buildStatus) {
case ACTOR_JOB_STATUSES.SUCCEEDED:
return { resultLabel: 'SUCCEEDED', exitCode: 0, ok: true };
case ACTOR_JOB_STATUSES.READY:
case ACTOR_JOB_STATUSES.RUNNING:
return { resultLabel: 'RUNNING', exitCode: 0, ok: true };
Comment on lines +75 to +77

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: might be a bit misleading. Why don't we have resultLabel: 'READY'?

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe the exitCode should be optional and not returned in case the process hasn't ended yet. Technically, at a time of READY and RUNNING, there is no exitCode yet. ok is alright, as at that time the process is truly ok.

case ACTOR_JOB_STATUSES.ABORTED:
case ACTOR_JOB_STATUSES.ABORTING:
return {
resultLabel: 'ABORTED',
exitCode: CommandExitCodes.BuildAborted,
ok: false,
errorMessage: 'Build aborted',
};
case ACTOR_JOB_STATUSES.TIMED_OUT:
case ACTOR_JOB_STATUSES.TIMING_OUT:
return {
resultLabel: 'TIMED_OUT',
exitCode: CommandExitCodes.BuildTimedOut,
ok: false,
errorMessage: 'Build timed out',
};

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: similarly as above, why don't we have distinguished resultLabel + errorMessage for both ABORTED and ABORTING (respectively TIMED_OUT and TIMING_OUT)?

default:
return { resultLabel: 'FAILED', exitCode: CommandExitCodes.BuildFailed, ok: false, errorMessage: 'Build failed' };

@DaveHanns DaveHanns Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: not super sure about the default. If there is ever a new state, it might fall back to FAILED even if it is not.

}
}

export class ActorsPushCommand extends ApifyCommand<typeof ActorsPushCommand> {
static override name = 'push' as const;

Expand Down Expand Up @@ -424,44 +472,68 @@ Skipping push. Use --force to override.`,
}
}

if (this.flags.json) {
printJsonToStdout(build);
return;
const buildStatus = build.status as string;
const outcome = resolvePushOutcome(buildStatus);

const actorUrl = `https://console.apify.com${redirectUrlPart}/actors/${build.actId}`;
const buildUrl = `${actorUrl}#/builds/${build.buildNumber}`;

// Surface the tail of the build log as the failure reason. Best-effort:
// the build status already conveys the outcome if the log can't be read.
let logTail: string[] = [];
if (outcome.errorMessage) {
try {
const log = await apifyClient.log(build.id).get();
if (log) {
logTail = log
.split('\n')
.map((line) => line.trimEnd())
.filter((line) => line.length > 0)
.slice(-BUILD_LOG_TAIL_LINES);
}
} catch {
// ignore — reason block is optional
}
}

link({
message: 'Actor build detail',
url: `https://console.apify.com${redirectUrlPart}/actors/${build.actId}#/builds/${build.buildNumber}`,
});
if (outcome.exitCode !== 0) {
process.exitCode = outcome.exitCode;
}

link({
message: 'Actor detail',
url: `https://console.apify.com${redirectUrlPart}/actors/${build.actId}`,
});
const result: PushResult = {
ok: outcome.ok,
operation: 'push',
actor: { id: build.actId, url: actorUrl },
build: { id: build.id, number: build.buildNumber, status: buildStatus, url: buildUrl },
exitCode: outcome.exitCode,
};
if (outcome.errorMessage) {
result.error = { phase: 'build', message: outcome.errorMessage, logTail };
}

if (this.flags.open) {
await open(`https://console.apify.com${redirectUrlPart}/actors/${build.actId}`);
if (this.flags.json) {
printJsonToStdout(result);
return;
}

if (build.status === ACTOR_JOB_STATUSES.SUCCEEDED) {
success({ message: 'Actor was deployed to Apify cloud and built there.' });
// @ts-expect-error FIX THESE TYPES 😢
} else if (build.status === ACTOR_JOB_STATUSES.READY) {
warning({ message: 'Build is waiting for allocation.' });
// @ts-expect-error FIX THESE TYPES 😢
} else if (build.status === ACTOR_JOB_STATUSES.RUNNING) {
warning({ message: 'Build is still running.' });
// @ts-expect-error FIX THESE TYPES 😢
} else if (build.status === ACTOR_JOB_STATUSES.ABORTED || build.status === ACTOR_JOB_STATUSES.ABORTING) {
warning({ message: 'Build was aborted!' });
process.exitCode = CommandExitCodes.BuildAborted;
// @ts-expect-error FIX THESE TYPES 😢
} else if (build.status === ACTOR_JOB_STATUSES.TIMED_OUT || build.status === ACTOR_JOB_STATUSES.TIMING_OUT) {
warning({ message: 'Build timed out!' });
process.exitCode = CommandExitCodes.BuildTimedOut;
} else {
error({ message: 'Build failed!' });
process.exitCode = CommandExitCodes.BuildFailed;
const lines = [
`Apify push result: ${outcome.resultLabel}`,
'',
'Upload: SUCCEEDED',
`Build: ${buildStatus}`,
`Actor ID: ${build.actId}`,
`Build ID: ${build.id}`,
`Build number: ${build.buildNumber}`,
...(outcome.exitCode !== 0 ? [`Exit code: ${outcome.exitCode}`] : []),
'',
`Actor URL: ${actorUrl}`,
`Build URL: ${buildUrl}`,
...(outcome.errorMessage && logTail.length ? ['', 'Reason:', ...logTail] : []),
];
simpleLog({ stdout: true, message: lines.join('\n') });

if (this.flags.open) {
await open(actorUrl);
}
}
}
28 changes: 28 additions & 0 deletions test/local/commands/push.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { ACTOR_JOB_STATUSES } from '@apify/consts';

import { resolvePushOutcome } from '../../../src/commands/actors/push.js';
import { CommandExitCodes } from '../../../src/lib/consts.js';

describe('resolvePushOutcome', () => {
test.each([
[ACTOR_JOB_STATUSES.SUCCEEDED, { resultLabel: 'SUCCEEDED', exitCode: 0, ok: true }],
[ACTOR_JOB_STATUSES.READY, { resultLabel: 'RUNNING', exitCode: 0, ok: true }],
[ACTOR_JOB_STATUSES.RUNNING, { resultLabel: 'RUNNING', exitCode: 0, ok: true }],
[ACTOR_JOB_STATUSES.ABORTED, { resultLabel: 'ABORTED', exitCode: CommandExitCodes.BuildAborted, ok: false }],
[ACTOR_JOB_STATUSES.ABORTING, { resultLabel: 'ABORTED', exitCode: CommandExitCodes.BuildAborted, ok: false }],
[ACTOR_JOB_STATUSES.TIMED_OUT, { resultLabel: 'TIMED_OUT', exitCode: CommandExitCodes.BuildTimedOut, ok: false }],
[ACTOR_JOB_STATUSES.TIMING_OUT, { resultLabel: 'TIMED_OUT', exitCode: CommandExitCodes.BuildTimedOut, ok: false }],
[ACTOR_JOB_STATUSES.FAILED, { resultLabel: 'FAILED', exitCode: CommandExitCodes.BuildFailed, ok: false }],
['SOME_UNKNOWN_STATUS', { resultLabel: 'FAILED', exitCode: CommandExitCodes.BuildFailed, ok: false }],
])('maps build status %s to the expected outcome', (status, expected) => {
expect(resolvePushOutcome(status)).toMatchObject(expected);
});

test('failing outcomes carry an error message, successful ones do not', () => {
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.SUCCEEDED).errorMessage).toBeUndefined();
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.RUNNING).errorMessage).toBeUndefined();
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.ABORTED).errorMessage).toBe('Build aborted');
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.TIMED_OUT).errorMessage).toBe('Build timed out');
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.FAILED).errorMessage).toBe('Build failed');
});
});
Loading