Skip to content

Commit f745e85

Browse files
authored
fix(telemetry): prevent deploy success with warning from recording as failure (#2769)
* chore(telemetry): model errors for legacy deploys * fix(telemetry): prevent deploy success with warning from recording as failure in metrics * fix(dev): continue harness session after legacy deploy warnings
1 parent 272585d commit f745e85

10 files changed

Lines changed: 123 additions & 14 deletions

File tree

‎src/errors/errors.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,23 @@ export class AgentCoreCLIError extends Error {
7777
/** Base for CLI errors intentionally omitted from root stderr output. */
7878
export class SilentCLIError extends AgentCoreCLIError {}
7979

80+
/** A v0 deploy completed, but its post-deploy steps reported warnings. */
81+
export class LegacyDeployWarningExit extends SilentCLIError {
82+
constructor() {
83+
super("v0 deploy completed with post-deploy warnings", { exitCode: 2 });
84+
}
85+
}
86+
87+
/** A v0 deploy failed after taking ownership of terminal output. */
88+
export class LegacyDeployError extends SilentCLIError {
89+
constructor(exitCode: number) {
90+
super(`v0 deploy exited unsuccessfully with code ${exitCode}`, {
91+
exitCode,
92+
meta: { legacyDeployExitCode: exitCode },
93+
});
94+
}
95+
}
96+
8097
/** Error raised for invalid user input. */
8198
export class InputValidationError extends AgentCoreCLIError {
8299
constructor(message?: string, options?: Omit<AgentCoreCLIErrorOptions, "source">) {

‎src/errors/index.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ export {
77
GatewayInvokeResponseError,
88
InputValidationError,
99
InvalidEnvironmentError,
10+
LegacyDeployError,
11+
LegacyDeployWarningExit,
1012
MalformedServiceResponseError,
1113
NestedProjectError,
1214
NetworkingError,

‎src/handlers/project/deploy/index.test.ts‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { afterEach, describe, expect, test } from "bun:test";
22
import { readFile, writeFile } from "node:fs/promises";
33
import { join } from "node:path";
4-
import { UserCancellationError } from "../../../errors/errors";
4+
import {
5+
LegacyDeployError,
6+
LegacyDeployWarningExit,
7+
UserCancellationError,
8+
} from "../../../errors/errors";
59
import { createRootHandler } from "../../index";
610
import { DEFAULT_GLOBAL_CONFIG } from "../../../globalConfig";
711
import { ValueContext } from "../../../router";
@@ -344,7 +348,52 @@ describe("project deploy handler", () => {
344348
);
345349
const subject = await testDeployCommand({ outputs: {} }, [], { runV0Deploy: async () => 7 });
346350

347-
await expect(subject.run(["--yes"])).rejects.toMatchObject({ exitCode: 7 });
351+
const error = await subject.run(["--yes"]).then(
352+
() => undefined,
353+
(caught: unknown) => caught,
354+
);
355+
expect(error).toBeInstanceOf(LegacyDeployError);
356+
expect(error).toMatchObject({ exitCode: 7, meta: { legacyDeployExitCode: 7 } });
357+
await assertMetricEmitted(subject.telemetryFilePath, "cli.command_run", {
358+
command_path: "/agentcore/deploy",
359+
exit_reason: "failure",
360+
error_name: "LegacyDeployError",
361+
legacy_deploy_exit_code: 7,
362+
});
363+
expect(subject.calls).toEqual([]);
364+
});
365+
366+
test("preserves v0's warning exit code without reporting a failed deploy", async () => {
367+
await inProjectWithTargets();
368+
await writeFile(
369+
join(process.cwd(), "agentcore", "agentcore.json"),
370+
JSON.stringify({ name: "orders", version: 1 }),
371+
);
372+
const subject = await testDeployCommand({ outputs: {} }, [], { runV0Deploy: async () => 2 });
373+
374+
const error = await subject.run(["--yes"]).then(
375+
() => undefined,
376+
(caught: unknown) => caught,
377+
);
378+
expect(error).toBeInstanceOf(LegacyDeployWarningExit);
379+
expect(error).toMatchObject({
380+
name: "LegacyDeployWarningExit",
381+
exitCode: 2,
382+
});
383+
await assertMetricEmitted(subject.telemetryFilePath, "cli.command_run", {
384+
command_path: "/agentcore/deploy",
385+
exit_reason: "success",
386+
legacy_deploy_exit_code: 2,
387+
});
388+
const events = (await readFile(subject.telemetryFilePath, "utf8"))
389+
.trim()
390+
.split("\n")
391+
.map((line) => JSON.parse(line));
392+
const warningRun = events.find(
393+
(event) =>
394+
event.metricName === "cli.command_run" && event.attrs.legacy_deploy_exit_code === 2,
395+
);
396+
expect(warningRun?.attrs).not.toHaveProperty("error_name");
348397
expect(subject.calls).toEqual([]);
349398
});
350399

‎src/handlers/project/deploy/index.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import z from "zod";
33
import { enclosingProjectRoot, isV0Project } from "../../../core/project/fsUtils";
44
import { runV0Deploy, type V0DeployRunner } from "../../../core/project/legacy/deploy";
55
import { assertNoPendingMigration } from "../../../core/project/migrate/journal";
6-
import { ProjectStateError, SilentCLIError, UserCancellationError } from "../../../errors";
6+
import {
7+
LegacyDeployError,
8+
LegacyDeployWarningExit,
9+
ProjectStateError,
10+
UserCancellationError,
11+
} from "../../../errors";
712
import type { AppIO } from "../../../io";
813
import { DEFAULT_TARGET_NAME } from "../../../projectSchemas/aws-targets";
914
import {
@@ -258,9 +263,9 @@ async function delegateV0Deploy(
258263
);
259264
}
260265
const exitCode = await (config.runV0Deploy ?? runV0Deploy)(input);
261-
if (exitCode !== 0) {
262-
throw new SilentCLIError("v0 deploy exited unsuccessfully", { exitCode });
263-
}
266+
ctx.value(CommandRunMetricEventKey)?.setAttributes({ legacy_deploy_exit_code: exitCode });
267+
if (exitCode === 2) throw new LegacyDeployWarningExit();
268+
if (exitCode !== 0) throw new LegacyDeployError(exitCode);
264269
}
265270

266271
/**

‎src/handlers/project/dev/index.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { JsonRendererKey } from "../../../tui";
2424
import { JsonKey, RegionKey } from "../../keys";
2525
import type { DeployProjectInput, Project, ProjectManager } from "../types";
2626
import type { V0DeployInput } from "../../../core/project/legacy/deploy";
27+
import type { V0DeployRunner } from "../../../core/project/legacy/deploy";
2728
import { ProjectSpecSchema } from "../../../projectSchemas/project";
2829
import {
2930
BMA_POLICY_FILE,
@@ -125,6 +126,7 @@ type HarnessOptions = {
125126
loadEnvironment?: DevProjectHandlerConfig["loadDevEnvironment"];
126127
/** Deploy through the real project manager instead of recording deployments. */
127128
realDeploy?: boolean;
129+
runV0Deploy?: V0DeployRunner;
128130
/**
129131
* Replaces the build-settings sync. By default it passes the project through,
130132
* since most projects here are in-memory; `"real"` uses the project manager's.
@@ -209,7 +211,7 @@ function harness(options: HarnessOptions = {}) {
209211
projectManager,
210212
runV0Deploy: async (input) => {
211213
legacyDeployments.push(input);
212-
return 0;
214+
return options.runV0Deploy ? options.runV0Deploy(input) : 0;
213215
},
214216
waitReady: async () => {
215217
await Bun.sleep(5);
@@ -877,6 +879,25 @@ describe("project dev harnesses", () => {
877879
);
878880
});
879881

882+
test("v0 post-deploy warnings still allow harness Inspector to start", async () => {
883+
const configuredProject = harnessProject();
884+
configuredProject.legacy = { format: "v0", v0Spec: {} as never };
885+
const subject = harness({ project: configuredProject, runV0Deploy: async () => 2 });
886+
const pending = subject.route(["--mode", "browser", "--target", "staging"]);
887+
pending.catch(() => undefined);
888+
await Bun.sleep(10);
889+
890+
try {
891+
expect(subject.legacyDeployments).toHaveLength(1);
892+
expect(subject.ui.starts).toEqual([{ port: 8081 }]);
893+
expect(subject.io.stderr()).toContain("Agent Inspector running at http://127.0.0.1:8081");
894+
} finally {
895+
process.emit("SIGTERM", "SIGTERM");
896+
await expect(pending).rejects.toBeInstanceOf(UserCancellationError);
897+
}
898+
expect(subject.ui.closed).toBe(1);
899+
});
900+
880901
test("harness dev's deploy refuses a target a migration hasn't finished", async () => {
881902
const root = await mkdtemp(join(tmpdir(), "agentcore-dev-migrate-"));
882903
try {

‎src/handlers/project/dev/index.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
ERROR_SOURCE,
1414
InputValidationError,
1515
DeserializationError,
16+
LegacyDeployWarningExit,
1617
NotImplementedError,
1718
ResourceNotFoundError,
1819
SilentCLIError,
@@ -257,11 +258,16 @@ async function runHarnessDev({
257258
}): Promise<void> {
258259
const { harnesses, deploymentTarget } = input;
259260
if (!input.skipDeploy) {
260-
await createDeployProjectHandler(config).handle(
261-
ctx,
262-
{ target: deploymentTarget, yes: input.yes },
263-
{},
264-
);
261+
try {
262+
await createDeployProjectHandler(config).handle(
263+
ctx,
264+
{ target: deploymentTarget, yes: input.yes },
265+
{},
266+
);
267+
} catch (error) {
268+
// v0 deploy completed; its post-deploy warnings must not prevent harness dev from starting.
269+
if (!(error instanceof LegacyDeployWarningExit)) throw error;
270+
}
265271
}
266272
if (input.mode === "headless" && (input.skipDeploy || ctx.require(JsonKey))) {
267273
config.io.stderr.write(`next step:\n ${DEPLOY_NEXT_STEP}\n`);

‎src/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { runWithExitCode } from "./runnable";
2020
import { DefaultGlobalConfigAccessor } from "./globalConfig";
2121
import { DefaultTelemetryClient, printFirstRunNotice } from "./telemetry";
2222
import { AgentCoreCLIError } from "./errors";
23+
import { shouldRecordCommandFailure } from "./telemetry/commandOutcome";
2324
import { PACKAGE_VERSION } from "./constants";
2425
import { CommandRunMetricEventKey, ValueContext } from "./router";
2526
import { NpmCliVersionManager, printUpdateNotice } from "./cliVersionManager";
@@ -116,7 +117,7 @@ process.exit(
116117
await rootHandler.route(argv, context);
117118
} catch (e) {
118119
const error = AgentCoreCLIError.fromError(e);
119-
if (error.exitCode !== 0) {
120+
if (shouldRecordCommandFailure(error)) {
120121
rootLogger.child({ error: error.json() }).error();
121122
commandRunMetricEvent.setAttributes({
122123
exit_reason: "failure",

‎src/telemetry/commandOutcome.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { AgentCoreCLIError, LegacyDeployWarningExit } from "../errors";
2+
3+
/** The v0 deployer uses exit 2 after a successful deploy with post-deploy warnings. */
4+
export function shouldRecordCommandFailure(error: AgentCoreCLIError): boolean {
5+
return error.exitCode !== 0 && !(error instanceof LegacyDeployWarningExit);
6+
}

‎src/telemetry/shapes.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export const commandRunSchema = safeSchema({
6464
error_source: z.enum(Object.values(ERROR_SOURCE)).optional(),
6565
is_tui: z.boolean().default(false),
6666
project_format: z.enum(["v0", "v1", "unknown"]).optional(),
67+
legacy_deploy_exit_code: z.number().int().min(0).max(255).optional(),
6768
project_runtime_count: z.number().int().nonnegative().optional(),
6869
project_memory_count: z.number().int().nonnegative().optional(),
6970
project_knowledge_base_count: z.number().int().nonnegative().optional(),

‎src/testing/telemetry.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { PACKAGE_VERSION } from "../constants";
66
import { AgentCoreCLIError } from "../errors/errors";
77
import { CommandRunMetricEventKey, type Context } from "../router";
88
import { DefaultTelemetryClient } from "../telemetry";
9+
import { shouldRecordCommandFailure } from "../telemetry/commandOutcome";
910
import { FileSystemSink } from "../telemetry/fileSystemSink";
1011
import type { AttributesOf, MetricName, TelemetryClient } from "../telemetry/types";
1112
import { TestGlobalConfigAccessor } from "./globalConfig";
@@ -77,7 +78,7 @@ export async function withTelemetry<T>(
7778
return await command(ctx.withValue(CommandRunMetricEventKey, event));
7879
} catch (cause) {
7980
const error = AgentCoreCLIError.fromError(cause);
80-
if (error.exitCode !== 0) {
81+
if (shouldRecordCommandFailure(error)) {
8182
event.setAttributes({
8283
exit_reason: "failure",
8384
error_name: error.name,

0 commit comments

Comments
 (0)