From 7a9bd18ba2e37e76912e4704126a354032bdc609 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Wed, 10 Apr 2024 17:58:51 +0100 Subject: [PATCH] v3: stop swallowing deployment errors and display them better (#1020) * exit worker with code 111 for handled errors * task monitor will ignore previously handled errors * fix logging of index errors * deployment error component * changeset --- .changeset/tender-oranges-rhyme.md | 5 +++ apps/kubernetes-provider/src/taskMonitor.ts | 8 +++- .../components/runs/v3/DeploymentError.tsx | 39 +++++++++++++++++++ .../v3/DeploymentPresenter.server.ts | 8 +++- .../route.tsx | 21 +--------- .../cli-v3/src/workers/prod/entry-point.ts | 38 +++++++++++------- 6 files changed, 83 insertions(+), 36 deletions(-) create mode 100644 .changeset/tender-oranges-rhyme.md create mode 100644 apps/webapp/app/components/runs/v3/DeploymentError.tsx diff --git a/.changeset/tender-oranges-rhyme.md b/.changeset/tender-oranges-rhyme.md new file mode 100644 index 000000000..cdd464549 --- /dev/null +++ b/.changeset/tender-oranges-rhyme.md @@ -0,0 +1,5 @@ +--- +"trigger.dev": patch +--- + +Stop swallowing deployment errors and display them better diff --git a/apps/kubernetes-provider/src/taskMonitor.ts b/apps/kubernetes-provider/src/taskMonitor.ts index c1d768432..1b2a61e4f 100644 --- a/apps/kubernetes-provider/src/taskMonitor.ts +++ b/apps/kubernetes-provider/src/taskMonitor.ts @@ -136,6 +136,13 @@ export class TaskMonitor { const podStatus = this.#getPodStatusSummary(pod.status); const containerState = this.#getContainerStateSummary(containerStatus.state); + const exitCode = containerState.exitCode ?? -1; + + // We use this special exit code to signal any errors were already handled elsewhere + if (exitCode === 111) { + return; + } + const rawLogs = await this.#getLogTail(podName); this.#logger.log(`${podName} failed with:`, { @@ -144,7 +151,6 @@ export class TaskMonitor { rawLogs, }); - const exitCode = containerState.exitCode ?? -1; const rawReason = podStatus.reason ?? containerState.reason ?? ""; const message = podStatus.message ?? containerState.message ?? ""; diff --git a/apps/webapp/app/components/runs/v3/DeploymentError.tsx b/apps/webapp/app/components/runs/v3/DeploymentError.tsx new file mode 100644 index 000000000..17fa14b8e --- /dev/null +++ b/apps/webapp/app/components/runs/v3/DeploymentError.tsx @@ -0,0 +1,39 @@ +import { CodeBlock } from "~/components/code/CodeBlock"; +import { Callout } from "~/components/primitives/Callout"; +import { Header2 } from "~/components/primitives/Headers"; +import type { ErrorData } from "~/presenters/v3/DeploymentPresenter.server"; + +type DeploymentErrorProps = { + errorData: ErrorData; +}; + +export function DeploymentError({ errorData }: DeploymentErrorProps) { + return ( +
+ + {errorData.message && {errorData.message}} + {errorData.stack && ( + + )} +
+ ); +} + +function DeploymentErrorHeader({ + title, + titleClassName, +}: { + title: string; + titleClassName?: string; +}) { + return ( +
+ {title} +
+ ); +} diff --git a/apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts b/apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts index a7244ece7..40ed2ec4c 100644 --- a/apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts @@ -12,6 +12,12 @@ import { User } from "~/models/user.server"; import { safeJsonParse } from "~/utils/json"; import { getUsername } from "~/utils/username"; +export type ErrorData = { + name: string; + message: string; + stack?: string; +}; + export class DeploymentPresenter { #prismaClient: PrismaClient; @@ -133,7 +139,7 @@ export class DeploymentPresenter { }; } - #prepareErrorData(errorData: WorkerDeployment["errorData"]) { + #prepareErrorData(errorData: WorkerDeployment["errorData"]): ErrorData | undefined { if (!errorData) { return; } diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments.$deploymentParam/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments.$deploymentParam/route.tsx index 02ee41dd3..0421c19f5 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments.$deploymentParam/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments.$deploymentParam/route.tsx @@ -19,6 +19,7 @@ import { TableHeaderCell, TableRow, } from "~/components/primitives/Table"; +import { DeploymentError } from "~/components/runs/v3/DeploymentError"; import { DeploymentStatus } from "~/components/runs/v3/DeploymentStatus"; import { TaskFunctionName } from "~/components/runs/v3/TaskPath"; import { useOrganization } from "~/hooks/useOrganizations"; @@ -158,25 +159,7 @@ export default function Page() { ) : deployment.errorData ? ( -
- {deployment.errorData.stack ? ( - - ) : ( -
- - {deployment.errorData.message} - -
- )} -
+ ) : null} diff --git a/packages/cli-v3/src/workers/prod/entry-point.ts b/packages/cli-v3/src/workers/prod/entry-point.ts index c5fe5fe98..745129ee1 100644 --- a/packages/cli-v3/src/workers/prod/entry-point.ts +++ b/packages/cli-v3/src/workers/prod/entry-point.ts @@ -417,7 +417,10 @@ class ProdWorker { } } catch (e) { if (e instanceof TaskMetadataParseError) { - logger.error("tasks metadata parse error", { message: e.zodIssues, tasks: e.tasks }); + logger.error("tasks metadata parse error", { + zodIssues: e.zodIssues, + tasks: e.tasks, + }); socket.emit("INDEXING_FAILED", { version: "v1", @@ -429,31 +432,35 @@ class ProdWorker { }, }); } else if (e instanceof UncaughtExceptionError) { - logger.error("uncaught exception", { message: e.originalError.message }); + const error = { + name: e.originalError.name, + message: e.originalError.message, + stack: e.originalError.stack, + }; + + logger.error("uncaught exception", { originalError: error }); socket.emit("INDEXING_FAILED", { version: "v1", deploymentId: this.deploymentId, - error: { - name: e.originalError.name, - message: e.originalError.message, - stack: e.originalError.stack, - }, + error, }); } else if (e instanceof Error) { - logger.error("error", { message: e.message }); + const error = { + name: e.name, + message: e.message, + stack: e.stack, + }; + + logger.error("error", { error }); socket.emit("INDEXING_FAILED", { version: "v1", deploymentId: this.deploymentId, - error: { - name: e.name, - message: e.message, - stack: e.stack, - }, + error, }); } else if (typeof e === "string") { - logger.error("string error", { message: e }); + logger.error("string error", { error: { message: e } }); socket.emit("INDEXING_FAILED", { version: "v1", @@ -477,7 +484,8 @@ class ProdWorker { } await setTimeout(200); - process.exit(1); + // Use exit code 111 so we can ignore those failures in the task monitor + process.exit(111); } }