Skip to content

v3: various small fixes #1192

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.

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

Merged
merged 7 commits into from
Jun 29, 2024
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
5 changes: 5 additions & 0 deletions .changeset/purple-spiders-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"trigger.dev": patch
---

Await file watcher cleanup in dev
9 changes: 9 additions & 0 deletions .changeset/tall-masks-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@trigger.dev/core-apps": patch
"trigger.dev": patch
"@trigger.dev/core": patch
---

- Fix artifact detection logs
- Fix OOM detection and error messages
- Add test link to cli deployment completion
16 changes: 8 additions & 8 deletions apps/kubernetes-provider/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,27 +529,27 @@ provider.listen();

const taskMonitor = new TaskMonitor({
runtimeEnv: RUNTIME_ENV,
onIndexFailure: async (deploymentId, failureInfo) => {
logger.log("Indexing failed", { deploymentId, failureInfo });
onIndexFailure: async (deploymentId, details) => {
logger.log("Indexing failed", { deploymentId, details });

try {
provider.platformSocket.send("INDEXING_FAILED", {
deploymentId,
error: {
name: `Crashed with exit code ${failureInfo.exitCode}`,
message: failureInfo.reason,
stack: failureInfo.logs,
name: `Crashed with exit code ${details.exitCode}`,
message: details.reason,
stack: details.logs,
},
});
} catch (error) {
logger.error(error);
}
},
onRunFailure: async (runId, failureInfo) => {
logger.log("Run failed:", { runId, failureInfo });
onRunFailure: async (runId, details) => {
logger.log("Run failed:", { runId, details });

try {
provider.platformSocket.send("WORKER_CRASHED", { runId, ...failureInfo });
provider.platformSocket.send("WORKER_CRASHED", { runId, ...details });
} catch (error) {
logger.error(error);
}
Expand Down
42 changes: 20 additions & 22 deletions apps/kubernetes-provider/src/taskMonitor.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
import * as k8s from "@kubernetes/client-node";
import { SimpleLogger } from "@trigger.dev/core-apps";
import { EXIT_CODE_ALREADY_HANDLED, EXIT_CODE_CHILD_NONZERO } from "@trigger.dev/core-apps/process";
import { setTimeout } from "timers/promises";
import PQueue from "p-queue";
import type { Prettify } from "@trigger.dev/core/v3";

type IndexFailureHandler = (
deploymentId: string,
failureInfo: {
exitCode: number;
reason: string;
logs: string;
}
) => Promise<any>;

type RunFailureHandler = (
runId: string,
failureInfo: {
exitCode: number;
reason: string;
logs: string;
}
) => Promise<any>;
type FailureDetails = Prettify<{
exitCode: number;
reason: string;
logs: string;
overrideCompletion: boolean;
}>;

type IndexFailureHandler = (deploymentId: string, details: FailureDetails) => Promise<any>;

type RunFailureHandler = (runId: string, details: FailureDetails) => Promise<any>;

type TaskMonitorOptions = {
runtimeEnv: "local" | "kubernetes";
Expand Down Expand Up @@ -144,8 +139,7 @@ export class TaskMonitor {
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) {
if (exitCode === EXIT_CODE_ALREADY_HANDLED) {
return;
}

Expand All @@ -162,6 +156,7 @@ export class TaskMonitor {

let reason = rawReason || "Unknown error";
let logs = rawLogs || "";
let overrideCompletion = false;

switch (rawReason) {
case "Error":
Expand All @@ -181,8 +176,10 @@ export class TaskMonitor {
}
break;
case "OOMKilled":
reason =
"Process ran out of memory! Try choosing a machine preset with more memory for this task.";
overrideCompletion = true;
reason = `${
exitCode === EXIT_CODE_CHILD_NONZERO ? "Child process" : "Parent process"
} ran out of memory! Try choosing a machine preset with more memory for this task.`;
break;
default:
break;
Expand All @@ -192,7 +189,8 @@ export class TaskMonitor {
exitCode,
reason,
logs,
};
overrideCompletion,
} satisfies FailureDetails;

const app = pod.metadata?.labels?.app;

Expand Down
2 changes: 1 addition & 1 deletion apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export function TaskRunsTable({
</TableHeader>
<TableBody>
{total === 0 && !hasFilters ? (
<TableBlankRow colSpan={9}>
<TableBlankRow colSpan={10}>
{!isLoading && <NoRuns title="No runs found" />}
</TableBlankRow>
) : runs.length === 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { Paragraph } from "~/components/primitives/Paragraph";
import { Property, PropertyTable } from "~/components/primitives/PropertyTable";
import {
Table,
TableBlankRow,
TableBody,
TableCell,
TableHeader,
Expand Down Expand Up @@ -180,6 +181,14 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
}
};

function PlaceholderText({ title }: { title: string }) {
return (
<div className="flex items-center justify-center">
<Paragraph className="w-auto">{title}</Paragraph>
</div>
);
}

export default function Page() {
const { schedule } = useTypedLoaderData<typeof loader>();
const location = useLocation();
Expand Down Expand Up @@ -252,18 +261,30 @@ export default function Page() {
</TableRow>
</TableHeader>
<TableBody>
{schedule.nextRuns.map((run, index) => (
<TableRow key={index}>
{!isUtc && (
<TableCell>
<DateTime date={run} timeZone={schedule.timezone} />
</TableCell>
)}
<TableCell>
<DateTime date={run} timeZone="UTC" />
</TableCell>
</TableRow>
))}
{schedule.active ? (
schedule.nextRuns.length ? (
schedule.nextRuns.map((run, index) => (
<TableRow key={index}>
{!isUtc && (
<TableCell>
<DateTime date={run} timeZone={schedule.timezone} />
</TableCell>
)}
<TableCell>
<DateTime date={run} timeZone="UTC" />
</TableCell>
</TableRow>
))
) : (
<TableBlankRow colSpan={1}>
<PlaceholderText title="You found a bug" />
</TableBlankRow>
)
) : (
<TableBlankRow colSpan={1}>
<PlaceholderText title="Schedule disabled" />
</TableBlankRow>
)}
</TableBody>
</Table>
</div>
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/v3/services/crashTaskRun.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type CrashTaskRunServiceOptions = {
logs?: string;
crashAttempts?: boolean;
crashedAt?: Date;
overrideCompletion?: boolean;
};

export class CrashTaskRunService extends BaseService {
Expand All @@ -36,7 +37,7 @@ export class CrashTaskRunService extends BaseService {
}

// Make sure the task run is in a crashable state
if (!isCrashableRunStatus(taskRun.status)) {
if (!opts.overrideCompletion && !isCrashableRunStatus(taskRun.status)) {
logger.error("Task run is not in a crashable state", { runId, status: taskRun.status });
return;
}
Expand Down
12 changes: 11 additions & 1 deletion apps/webapp/app/v3/services/deploymentIndexFailed.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ export class DeploymentIndexFailed extends BaseService {
message: string;
stack?: string;
stderr?: string;
}
},
overrideCompletion = false
) {
const isFriendlyId = maybeFriendlyId.startsWith("deployment_");

Expand All @@ -38,6 +39,15 @@ export class DeploymentIndexFailed extends BaseService {
}

if (FINAL_DEPLOYMENT_STATUSES.includes(deployment.status)) {
if (overrideCompletion) {
logger.error("No support for overriding final deployment statuses just yet", {
id: deployment.id,
status: deployment.status,
previousError: deployment.errorData,
incomingError: error,
});
}

logger.error("Worker deployment already in final state", {
id: deployment.id,
status: deployment.status,
Expand Down
9 changes: 8 additions & 1 deletion packages/cli-v3/src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,13 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
`${authorization.dashboardUrl}/projects/v3/${resolvedConfig.config.project}/deployments/${finishedDeployment.shortCode}`
);

const testLink = cliLink(
"Test tasks",
`${authorization.dashboardUrl}/projects/v3/${resolvedConfig.config.project}/test?environment=${
options.env === "prod" ? "prod" : "stg"
}`
);

switch (finishedDeployment.status) {
case "DEPLOYED": {
if (warnings.warnings.length > 0) {
Expand All @@ -461,7 +468,7 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
outro(
`Version ${version} deployed with ${taskCount} detected task${
taskCount === 1 ? "" : "s"
} ${deploymentLink}`
} | ${deploymentLink} | ${testLink}`
);
}

Expand Down
22 changes: 15 additions & 7 deletions packages/cli-v3/src/commands/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -760,15 +760,23 @@ function useDev({
});

return () => {
logger.debug(`Shutting down dev session for ${config.project}`);
const cleanup = async () => {
logger.debug(`Shutting down dev session for ${config.project}`);

taskFileWatcher.close();
const start = Date.now();

websocket?.close();
backgroundWorkerCoordinator.close();
ctx?.dispose().catch((error) => {
console.error(error);
});
await taskFileWatcher.close();

websocket?.close();
backgroundWorkerCoordinator.close();
ctx?.dispose().catch((error) => {
console.error(error);
});

logger.debug(`Shutdown completed in ${Date.now() - start}ms`);
};

cleanup();
};
}, [config, apiUrl, apiKey, environmentClient]);
}
Expand Down
1 change: 1 addition & 0 deletions packages/cli-v3/src/utilities/getUserPackageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export async function detectPackageManagerFromArtifacts(path: string): Promise<P
case LOCKFILES.npm:
case LOCKFILES.npmShrinkwrap:
logger.debug("Found npm artifact", { foundPath });
return "npm";
case LOCKFILES.bun:
logger.debug("Found bun artifact", { foundPath });
return "npm";
Expand Down
18 changes: 16 additions & 2 deletions packages/cli-v3/src/workers/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ export class GracefulExitTimeoutError extends Error {
export function getFriendlyErrorMessage(
code: number,
signal: NodeJS.Signals | null,
stderr: string | undefined
stderr: string | undefined,
dockerMode = true
) {
const message = (text: string) => {
if (signal) {
Expand All @@ -79,7 +80,20 @@ export function getFriendlyErrorMessage(
}
};

if (code === 137 || stderr?.includes("OOMErrorHandler")) {
if (code === 137) {
if (dockerMode) {
return message(
"Process ran out of memory! Try choosing a machine preset with more memory for this task."
);
} else {
// Note: containerState reason and message should be checked to clarify the error
return message(
"Process most likely ran out of memory, but we can't be certain. Try choosing a machine preset with more memory for this task."
);
}
}

if (stderr?.includes("OOMErrorHandler")) {
return message(
"Process ran out of memory! Try choosing a machine preset with more memory for this task."
);
Expand Down
Loading
Loading