Skip to content

Commit 5ae3da6

Browse files
authored
v3: various small fixes (#1192)
* Await file watcher cleanup in dev * Fix artifact detection logs * Fix next runs table when schedule disabled * Improve OOM error messages * Add test link to completed deployment message * Fix OOM detection, again * Add changeset
1 parent f565829 commit 5ae3da6

File tree

16 files changed

+155
-63
lines changed

16 files changed

+155
-63
lines changed

.changeset/purple-spiders-care.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"trigger.dev": patch
3+
---
4+
5+
Await file watcher cleanup in dev

.changeset/tall-masks-repeat.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@trigger.dev/core-apps": patch
3+
"trigger.dev": patch
4+
"@trigger.dev/core": patch
5+
---
6+
7+
- Fix artifact detection logs
8+
- Fix OOM detection and error messages
9+
- Add test link to cli deployment completion

apps/kubernetes-provider/src/index.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -529,27 +529,27 @@ provider.listen();
529529

530530
const taskMonitor = new TaskMonitor({
531531
runtimeEnv: RUNTIME_ENV,
532-
onIndexFailure: async (deploymentId, failureInfo) => {
533-
logger.log("Indexing failed", { deploymentId, failureInfo });
532+
onIndexFailure: async (deploymentId, details) => {
533+
logger.log("Indexing failed", { deploymentId, details });
534534

535535
try {
536536
provider.platformSocket.send("INDEXING_FAILED", {
537537
deploymentId,
538538
error: {
539-
name: `Crashed with exit code ${failureInfo.exitCode}`,
540-
message: failureInfo.reason,
541-
stack: failureInfo.logs,
539+
name: `Crashed with exit code ${details.exitCode}`,
540+
message: details.reason,
541+
stack: details.logs,
542542
},
543543
});
544544
} catch (error) {
545545
logger.error(error);
546546
}
547547
},
548-
onRunFailure: async (runId, failureInfo) => {
549-
logger.log("Run failed:", { runId, failureInfo });
548+
onRunFailure: async (runId, details) => {
549+
logger.log("Run failed:", { runId, details });
550550

551551
try {
552-
provider.platformSocket.send("WORKER_CRASHED", { runId, ...failureInfo });
552+
provider.platformSocket.send("WORKER_CRASHED", { runId, ...details });
553553
} catch (error) {
554554
logger.error(error);
555555
}

apps/kubernetes-provider/src/taskMonitor.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,20 @@
11
import * as k8s from "@kubernetes/client-node";
22
import { SimpleLogger } from "@trigger.dev/core-apps";
3+
import { EXIT_CODE_ALREADY_HANDLED, EXIT_CODE_CHILD_NONZERO } from "@trigger.dev/core-apps/process";
34
import { setTimeout } from "timers/promises";
45
import PQueue from "p-queue";
6+
import type { Prettify } from "@trigger.dev/core/v3";
57

6-
type IndexFailureHandler = (
7-
deploymentId: string,
8-
failureInfo: {
9-
exitCode: number;
10-
reason: string;
11-
logs: string;
12-
}
13-
) => Promise<any>;
14-
15-
type RunFailureHandler = (
16-
runId: string,
17-
failureInfo: {
18-
exitCode: number;
19-
reason: string;
20-
logs: string;
21-
}
22-
) => Promise<any>;
8+
type FailureDetails = Prettify<{
9+
exitCode: number;
10+
reason: string;
11+
logs: string;
12+
overrideCompletion: boolean;
13+
}>;
14+
15+
type IndexFailureHandler = (deploymentId: string, details: FailureDetails) => Promise<any>;
16+
17+
type RunFailureHandler = (runId: string, details: FailureDetails) => Promise<any>;
2318

2419
type TaskMonitorOptions = {
2520
runtimeEnv: "local" | "kubernetes";
@@ -144,8 +139,7 @@ export class TaskMonitor {
144139
const containerState = this.#getContainerStateSummary(containerStatus.state);
145140
const exitCode = containerState.exitCode ?? -1;
146141

147-
// We use this special exit code to signal any errors were already handled elsewhere
148-
if (exitCode === 111) {
142+
if (exitCode === EXIT_CODE_ALREADY_HANDLED) {
149143
return;
150144
}
151145

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

163157
let reason = rawReason || "Unknown error";
164158
let logs = rawLogs || "";
159+
let overrideCompletion = false;
165160

166161
switch (rawReason) {
167162
case "Error":
@@ -181,8 +176,10 @@ export class TaskMonitor {
181176
}
182177
break;
183178
case "OOMKilled":
184-
reason =
185-
"Process ran out of memory! Try choosing a machine preset with more memory for this task.";
179+
overrideCompletion = true;
180+
reason = `${
181+
exitCode === EXIT_CODE_CHILD_NONZERO ? "Child process" : "Parent process"
182+
} ran out of memory! Try choosing a machine preset with more memory for this task.`;
186183
break;
187184
default:
188185
break;
@@ -192,7 +189,8 @@ export class TaskMonitor {
192189
exitCode,
193190
reason,
194191
logs,
195-
};
192+
overrideCompletion,
193+
} satisfies FailureDetails;
196194

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

apps/webapp/app/components/runs/v3/TaskRunsTable.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export function TaskRunsTable({
125125
</TableHeader>
126126
<TableBody>
127127
{total === 0 && !hasFilters ? (
128-
<TableBlankRow colSpan={9}>
128+
<TableBlankRow colSpan={10}>
129129
{!isLoading && <NoRuns title="No runs found" />}
130130
</TableBlankRow>
131131
) : runs.length === 0 ? (

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.schedules.$scheduleParam/route.tsx

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { Paragraph } from "~/components/primitives/Paragraph";
2323
import { Property, PropertyTable } from "~/components/primitives/PropertyTable";
2424
import {
2525
Table,
26+
TableBlankRow,
2627
TableBody,
2728
TableCell,
2829
TableHeader,
@@ -180,6 +181,14 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
180181
}
181182
};
182183

184+
function PlaceholderText({ title }: { title: string }) {
185+
return (
186+
<div className="flex items-center justify-center">
187+
<Paragraph className="w-auto">{title}</Paragraph>
188+
</div>
189+
);
190+
}
191+
183192
export default function Page() {
184193
const { schedule } = useTypedLoaderData<typeof loader>();
185194
const location = useLocation();
@@ -252,18 +261,30 @@ export default function Page() {
252261
</TableRow>
253262
</TableHeader>
254263
<TableBody>
255-
{schedule.nextRuns.map((run, index) => (
256-
<TableRow key={index}>
257-
{!isUtc && (
258-
<TableCell>
259-
<DateTime date={run} timeZone={schedule.timezone} />
260-
</TableCell>
261-
)}
262-
<TableCell>
263-
<DateTime date={run} timeZone="UTC" />
264-
</TableCell>
265-
</TableRow>
266-
))}
264+
{schedule.active ? (
265+
schedule.nextRuns.length ? (
266+
schedule.nextRuns.map((run, index) => (
267+
<TableRow key={index}>
268+
{!isUtc && (
269+
<TableCell>
270+
<DateTime date={run} timeZone={schedule.timezone} />
271+
</TableCell>
272+
)}
273+
<TableCell>
274+
<DateTime date={run} timeZone="UTC" />
275+
</TableCell>
276+
</TableRow>
277+
))
278+
) : (
279+
<TableBlankRow colSpan={1}>
280+
<PlaceholderText title="You found a bug" />
281+
</TableBlankRow>
282+
)
283+
) : (
284+
<TableBlankRow colSpan={1}>
285+
<PlaceholderText title="Schedule disabled" />
286+
</TableBlankRow>
287+
)}
267288
</TableBody>
268289
</Table>
269290
</div>

apps/webapp/app/v3/services/crashTaskRun.server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export type CrashTaskRunServiceOptions = {
1313
logs?: string;
1414
crashAttempts?: boolean;
1515
crashedAt?: Date;
16+
overrideCompletion?: boolean;
1617
};
1718

1819
export class CrashTaskRunService extends BaseService {
@@ -36,7 +37,7 @@ export class CrashTaskRunService extends BaseService {
3637
}
3738

3839
// Make sure the task run is in a crashable state
39-
if (!isCrashableRunStatus(taskRun.status)) {
40+
if (!opts.overrideCompletion && !isCrashableRunStatus(taskRun.status)) {
4041
logger.error("Task run is not in a crashable state", { runId, status: taskRun.status });
4142
return;
4243
}

apps/webapp/app/v3/services/deploymentIndexFailed.server.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ export class DeploymentIndexFailed extends BaseService {
1818
message: string;
1919
stack?: string;
2020
stderr?: string;
21-
}
21+
},
22+
overrideCompletion = false
2223
) {
2324
const isFriendlyId = maybeFriendlyId.startsWith("deployment_");
2425

@@ -38,6 +39,15 @@ export class DeploymentIndexFailed extends BaseService {
3839
}
3940

4041
if (FINAL_DEPLOYMENT_STATUSES.includes(deployment.status)) {
42+
if (overrideCompletion) {
43+
logger.error("No support for overriding final deployment statuses just yet", {
44+
id: deployment.id,
45+
status: deployment.status,
46+
previousError: deployment.errorData,
47+
incomingError: error,
48+
});
49+
}
50+
4151
logger.error("Worker deployment already in final state", {
4252
id: deployment.id,
4353
status: deployment.status,

packages/cli-v3/src/commands/deploy.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,13 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
441441
`${authorization.dashboardUrl}/projects/v3/${resolvedConfig.config.project}/deployments/${finishedDeployment.shortCode}`
442442
);
443443

444+
const testLink = cliLink(
445+
"Test tasks",
446+
`${authorization.dashboardUrl}/projects/v3/${resolvedConfig.config.project}/test?environment=${
447+
options.env === "prod" ? "prod" : "stg"
448+
}`
449+
);
450+
444451
switch (finishedDeployment.status) {
445452
case "DEPLOYED": {
446453
if (warnings.warnings.length > 0) {
@@ -461,7 +468,7 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
461468
outro(
462469
`Version ${version} deployed with ${taskCount} detected task${
463470
taskCount === 1 ? "" : "s"
464-
} ${deploymentLink}`
471+
} | ${deploymentLink} | ${testLink}`
465472
);
466473
}
467474

packages/cli-v3/src/commands/dev.tsx

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -760,15 +760,23 @@ function useDev({
760760
});
761761

762762
return () => {
763-
logger.debug(`Shutting down dev session for ${config.project}`);
763+
const cleanup = async () => {
764+
logger.debug(`Shutting down dev session for ${config.project}`);
764765

765-
taskFileWatcher.close();
766+
const start = Date.now();
766767

767-
websocket?.close();
768-
backgroundWorkerCoordinator.close();
769-
ctx?.dispose().catch((error) => {
770-
console.error(error);
771-
});
768+
await taskFileWatcher.close();
769+
770+
websocket?.close();
771+
backgroundWorkerCoordinator.close();
772+
ctx?.dispose().catch((error) => {
773+
console.error(error);
774+
});
775+
776+
logger.debug(`Shutdown completed in ${Date.now() - start}ms`);
777+
};
778+
779+
cleanup();
772780
};
773781
}, [config, apiUrl, apiKey, environmentClient]);
774782
}

packages/cli-v3/src/utilities/getUserPackageManager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export async function detectPackageManagerFromArtifacts(path: string): Promise<P
6262
case LOCKFILES.npm:
6363
case LOCKFILES.npmShrinkwrap:
6464
logger.debug("Found npm artifact", { foundPath });
65+
return "npm";
6566
case LOCKFILES.bun:
6667
logger.debug("Found bun artifact", { foundPath });
6768
return "npm";

packages/cli-v3/src/workers/common/errors.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ export class GracefulExitTimeoutError extends Error {
6969
export function getFriendlyErrorMessage(
7070
code: number,
7171
signal: NodeJS.Signals | null,
72-
stderr: string | undefined
72+
stderr: string | undefined,
73+
dockerMode = true
7374
) {
7475
const message = (text: string) => {
7576
if (signal) {
@@ -79,7 +80,20 @@ export function getFriendlyErrorMessage(
7980
}
8081
};
8182

82-
if (code === 137 || stderr?.includes("OOMErrorHandler")) {
83+
if (code === 137) {
84+
if (dockerMode) {
85+
return message(
86+
"Process ran out of memory! Try choosing a machine preset with more memory for this task."
87+
);
88+
} else {
89+
// Note: containerState reason and message should be checked to clarify the error
90+
return message(
91+
"Process most likely ran out of memory, but we can't be certain. Try choosing a machine preset with more memory for this task."
92+
);
93+
}
94+
}
95+
96+
if (stderr?.includes("OOMErrorHandler")) {
8397
return message(
8498
"Process ran out of memory! Try choosing a machine preset with more memory for this task."
8599
);

0 commit comments

Comments
 (0)