Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
165 changes: 132 additions & 33 deletions src/commands/actors/push.ts
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,79 @@ 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;

// TODO: switch to `type` once the `consistent-type-definitions` lint rule is
// aligned with the Apify Coding Standards (tracked in
// https://github.com/apify/apify-cli/issues/1211).
interface PushResult {
Comment thread
DaveHanns marked this conversation as resolved.
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;
}
Comment thread
DaveHanns marked this conversation as resolved.

interface PushOutcome {
resultLabel: string;
exitCode?: number;
ok: boolean;
errorMessage?: string;
}

// Maps the final build status to the overall push outcome. A still-running
// fire-and-forget build is not a failure (`ok: true`) — its pending state is
// conveyed by the build status, and it carries no exit code yet.
export function resolvePushOutcome(buildStatus: string): PushOutcome {
switch (buildStatus) {
case ACTOR_JOB_STATUSES.SUCCEEDED:
return { resultLabel: 'SUCCEEDED', exitCode: 0, ok: true };
case ACTOR_JOB_STATUSES.READY:
return { resultLabel: 'PENDING', ok: true };
case ACTOR_JOB_STATUSES.RUNNING:
return { resultLabel: 'RUNNING', ok: true };
case ACTOR_JOB_STATUSES.ABORTING:
return {
resultLabel: 'ABORTING',
exitCode: CommandExitCodes.BuildAborted,
ok: false,
errorMessage: 'Build is aborting',
};
case ACTOR_JOB_STATUSES.ABORTED:
return {
resultLabel: 'ABORTED',
exitCode: CommandExitCodes.BuildAborted,
ok: false,
errorMessage: 'Build aborted',
};
case ACTOR_JOB_STATUSES.TIMING_OUT:
return {
resultLabel: 'TIMING_OUT',
exitCode: CommandExitCodes.BuildTimedOut,
ok: false,
errorMessage: 'Build is timing out',
};
case ACTOR_JOB_STATUSES.TIMED_OUT:
return {
resultLabel: 'TIMED_OUT',
exitCode: CommandExitCodes.BuildTimedOut,
ok: false,
errorMessage: 'Build timed out',
};
Comment thread
DaveHanns marked this conversation as resolved.
case ACTOR_JOB_STATUSES.FAILED:
return { resultLabel: 'FAILED', exitCode: CommandExitCodes.BuildFailed, ok: false, errorMessage: 'Build failed' };
Comment thread
DaveHanns marked this conversation as resolved.
default:
return {
resultLabel: 'UNKNOWN',
exitCode: CommandExitCodes.BuildFailed,
ok: false,
errorMessage: `Build finished with unexpected status "${buildStatus}"`,
};
}
}

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

Expand Down Expand Up @@ -424,44 +497,70 @@ 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}`;
Comment thread
DaveHanns marked this conversation as resolved.
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) {
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 },
};
if (outcome.exitCode !== undefined) {
result.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 = [

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: do we already implement same style logging in case the upload itself fails?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not now, It is not so big change but I would like to merge this PR and do it in followup, upload fails should not be common if platform is not in some serious problems (or user is in some isolated env).

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.

Makes sense 👍 You can either use the same issue (in that case don't close it with this PR) or create a new one for that once you tackle it.

`Apify push result: ${outcome.resultLabel}`,
'',
'Upload: SUCCEEDED',
`Build: ${buildStatus}`,
`Actor ID: ${build.actId}`,
`Build ID: ${build.id}`,
`Build number: ${build.buildNumber}`,
...(outcome.exitCode ? [`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);
}
}
}
39 changes: 39 additions & 0 deletions test/local/commands/push.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
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: 'PENDING', ok: true }],
[ACTOR_JOB_STATUSES.RUNNING, { resultLabel: 'RUNNING', ok: true }],
[ACTOR_JOB_STATUSES.ABORTING, { resultLabel: 'ABORTING', exitCode: CommandExitCodes.BuildAborted, ok: false }],
[ACTOR_JOB_STATUSES.ABORTED, { resultLabel: 'ABORTED', exitCode: CommandExitCodes.BuildAborted, ok: false }],
[ACTOR_JOB_STATUSES.TIMING_OUT, { resultLabel: 'TIMING_OUT', exitCode: CommandExitCodes.BuildTimedOut, ok: false }],
[ACTOR_JOB_STATUSES.TIMED_OUT, { resultLabel: 'TIMED_OUT', exitCode: CommandExitCodes.BuildTimedOut, ok: false }],
[ACTOR_JOB_STATUSES.FAILED, { resultLabel: 'FAILED', exitCode: CommandExitCodes.BuildFailed, ok: false }],
['SOME_UNKNOWN_STATUS', { resultLabel: 'UNKNOWN', exitCode: CommandExitCodes.BuildFailed, ok: false }],
])('maps build status %s to the expected outcome', (status, expected) => {
expect(resolvePushOutcome(status)).toMatchObject(expected);
});

test('in-progress builds carry no exit code yet, terminal ones do', () => {
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.READY).exitCode).toBeUndefined();
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.RUNNING).exitCode).toBeUndefined();
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.SUCCEEDED).exitCode).toBe(0);
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.ABORTING).exitCode).toBe(CommandExitCodes.BuildAborted);
expect(resolvePushOutcome(ACTOR_JOB_STATUSES.FAILED).exitCode).toBe(CommandExitCodes.BuildFailed);
});

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