diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 927709226..e9a0d9143 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -6,7 +6,12 @@ import type { Actor, ActorCollectionCreateOptions, ActorDefaultRunOptions } from import open from 'open'; import { fetchManifest } from '@apify/actor-templates'; -import { ACTOR_JOB_STATUSES, ACTOR_SOURCE_TYPES, MAX_MULTIFILE_BYTES } from '@apify/consts'; +import { + ACTOR_JOB_STATUSES, + ACTOR_JOB_TERMINAL_STATUSES, + ACTOR_SOURCE_TYPES, + MAX_MULTIFILE_BYTES, +} from '@apify/consts'; import { createHmacSignature } from '@apify/utilities'; import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; @@ -25,6 +30,7 @@ import { getLocalUserInfo, getLoggedClientOrThrow, outputJobLog, + parseWaitForFinishMillis, printJsonToStdout, } from '../../lib/utils.js'; @@ -190,9 +196,7 @@ export class ActorsPushCommand extends ApifyCommand { buildTag = DEFAULT_BUILD_TAG; } - const waitForFinishMillis = Number.isNaN(this.flags.waitForFinish) - ? undefined - : Number.parseInt(this.flags.waitForFinish!, 10) * 1000; + const waitForFinishMillis = parseWaitForFinishMillis(this.flags.waitForFinish); // User can override actorId of pushing Actor. // It causes that we push Actor to this id but attributes in localConfig will remain same. @@ -359,18 +363,16 @@ Skipping push. Use --force to override.`, waitForFinish: 2, // NOTE: We need to wait some time to Apify open stream and we can create connection }); - try { - // While the log is streaming, forward interrupt signals to a - // platform-side abort so the build doesn't keep running after the - // user gives up waiting (Ctrl+C, SIGTERM from a parent process, - // SIGHUP from a closing terminal). The `using` binding guarantees - // the listener is removed before we poll for final status. - using _signalHandler = useAbortJobOnSignal({ - apifyClient, - kind: 'build', - jobId: build.id, - }); + // Forward interrupt signals (Ctrl+C, SIGTERM, SIGHUP) to a platform-side + // abort for the lifetime of log streaming AND status polling, so the + // build doesn't keep running after the user gives up waiting. + using _signalHandler = useAbortJobOnSignal({ + apifyClient, + kind: 'build', + jobId: build.id, + }); + try { await outputJobLog({ job: build, timeoutMillis: waitForFinishMillis, apifyClient }); } catch (err) { warning({ message: 'Can not get log:' }); @@ -379,6 +381,42 @@ Skipping push. Use --force to override.`, build = (await apifyClient.build(build.id).get())!; + // `outputJobLog` can return before the build is actually terminal (stream + // ended early, timeout hit). Poll so the status branches below see the + // real outcome. With no --wait-for-finish, the flag documents "waits + // forever", so poll without a deadline. + const deadline = waitForFinishMillis === undefined ? Infinity : Date.now() + waitForFinishMillis; + while (!ACTOR_JOB_TERMINAL_STATUSES.includes(build.status as never) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 1000)); + build = (await apifyClient.build(build.id).get())!; + } + + // Platform updates `taggedBuilds[buildTag]` asynchronously after the + // build finishes. Wait until the tag points at this build so callers + // (including --json automation) that immediately + // `actor.start({ build: buildTag })` don't race it. Capped at 30s so an + // unknown platform delay can't stall push forever. + if (build.status === ACTOR_JOB_STATUSES.SUCCEEDED && buildTag) { + // 30s budget is independent of --wait-for-finish: the build is already + // done, we're only waiting on the platform to update the tag pointer. + const tagDeadline = Date.now() + 30_000; + let tagApplied = false; + while (Date.now() < tagDeadline) { + const a = await actorClient.get(); + if (a?.taggedBuilds?.[buildTag]?.buildId === build.id) { + tagApplied = true; + break; + } + await new Promise((resolve) => setTimeout(resolve, 1000)); + } + if (!tagApplied) { + warning({ + message: `Build succeeded but tag "${buildTag}" did not update within 30s; subsequent calls referencing this tag may race.`, + }); + process.exitCode = CommandExitCodes.BuildTimedOut; + } + } + if (this.flags.json) { printJsonToStdout(build); return; @@ -403,9 +441,11 @@ Skipping push. Use --force to override.`, // @ts-expect-error FIX THESE TYPES 😢 } else if (build.status === ACTOR_JOB_STATUSES.READY) { warning({ message: 'Build is waiting for allocation.' }); + process.exitCode = CommandExitCodes.BuildTimedOut; // @ts-expect-error FIX THESE TYPES 😢 } else if (build.status === ACTOR_JOB_STATUSES.RUNNING) { warning({ message: 'Build is still running.' }); + process.exitCode = CommandExitCodes.BuildTimedOut; // @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!' }); diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 852d1b242..a664a5a4f 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -607,7 +607,7 @@ export const outputJobLog = async ({ } }); - if (timeoutMillis) { + if (timeoutMillis !== undefined) { nodeTimeout = setTimeout(() => { stream.destroy(); resolve('timeouts'); @@ -840,3 +840,10 @@ export function shellConfigFile(userHomeDirectory: string, shell: ReturnType { + it('returns undefined when flag is omitted', () => { + expect(parseWaitForFinishMillis(undefined)).toBeUndefined(); + }); + + it('returns undefined for non-numeric input', () => { + expect(parseWaitForFinishMillis('abc')).toBeUndefined(); + }); + + it('returns 0 for zero (no wait)', () => { + expect(parseWaitForFinishMillis('0')).toBe(0); + }); + + it('returns undefined for negative values', () => { + expect(parseWaitForFinishMillis('-5')).toBeUndefined(); + }); + + it('converts positive seconds to milliseconds', () => { + expect(parseWaitForFinishMillis('30')).toBe(30_000); + }); +});