Skip to content

Commit 914ceaf

Browse files
authored
dev CLI: cleanup deprecated background worker files (fixes #1572) (#1595)
* dev CLI: cleanup deprecated background worker files (fixes #1572) * Add changeset
1 parent 472e5f9 commit 914ceaf

File tree

6 files changed

+66
-38
lines changed

6 files changed

+66
-38
lines changed

.changeset/unlucky-meals-develop.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"trigger.dev": patch
3+
---
4+
5+
cleanup deprecated background worker files (fixes #1572)

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

+6
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const DevCommandOptions = CommonCommandOptions.extend({
1818
projectRef: z.string().optional(),
1919
skipUpdateCheck: z.boolean().default(false),
2020
envFile: z.string().optional(),
21+
keepTmpFiles: z.boolean().default(false),
2122
});
2223

2324
export type DevCommandOptions = z.infer<typeof DevCommandOptions>;
@@ -38,6 +39,10 @@ export function configureDevCommand(program: Command) {
3839
)
3940
.option("--debug-otel", "Enable OpenTelemetry debugging")
4041
.option("--skip-update-check", "Skip checking for @trigger.dev package updates")
42+
.option(
43+
"--keep-tmp-files",
44+
"Keep temporary files after the dev session ends, helpful for debugging"
45+
)
4146
).action(async (options) => {
4247
wrapCommandAction("dev", DevCommandOptions, options, async (opts) => {
4348
await devCommand(opts);
@@ -151,6 +156,7 @@ async function startDev(options: StartDevOptions) {
151156
initialMode: "local",
152157
dashboardUrl: options.login.dashboardUrl,
153158
showInteractiveDevSession: true,
159+
keepTmpFiles: options.keepTmpFiles,
154160
});
155161
}
156162

packages/cli-v3/src/dev/backgroundWorker.ts

+13-30
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ export class BackgroundWorkerCoordinator {
111111
}
112112

113113
this._backgroundWorkers.set(worker.serverWorker.id, worker);
114+
114115
this.onWorkerRegistered.post({
115116
worker,
116117
id: worker.serverWorker.id,
@@ -126,14 +127,6 @@ export class BackgroundWorkerCoordinator {
126127
});
127128
}
128129

129-
close() {
130-
for (const worker of this._backgroundWorkers.values()) {
131-
worker.close();
132-
}
133-
134-
this._backgroundWorkers.clear();
135-
}
136-
137130
async executeTaskRun(id: string, payload: TaskRunExecutionPayload, messageId: string) {
138131
const worker = this._backgroundWorkers.get(id);
139132

@@ -186,11 +179,11 @@ export class BackgroundWorkerCoordinator {
186179
export type BackgroundWorkerOptions = {
187180
env: Record<string, string>;
188181
cwd: string;
182+
stop: () => void;
189183
};
190184

191185
export class BackgroundWorker {
192186
public onTaskRunHeartbeat: Evt<string> = new Evt();
193-
private _onClose: Evt<void> = new Evt();
194187

195188
public deprecated: boolean = false;
196189
public manifest: WorkerManifest | undefined;
@@ -199,33 +192,27 @@ export class BackgroundWorker {
199192
_taskRunProcesses: Map<string, TaskRunProcess> = new Map();
200193
private _taskRunProcessesBeingKilled: Map<number, TaskRunProcess> = new Map();
201194

202-
private _closed: boolean = false;
203-
204195
constructor(
205196
public build: BuildManifest,
206197
public params: BackgroundWorkerOptions
207198
) {}
208199

209200
deprecate() {
210-
this.deprecated = true;
211-
}
212-
213-
close() {
214-
if (this._closed) {
201+
if (this.deprecated) {
215202
return;
216203
}
217204

218-
this._closed = true;
205+
this.deprecated = true;
219206

220-
this.onTaskRunHeartbeat.detach();
207+
this.#tryStopWorker();
208+
}
221209

222-
// We need to close all the task run processes
223-
for (const taskRunProcess of this._taskRunProcesses.values()) {
224-
taskRunProcess.cleanup(true);
225-
}
210+
#tryStopWorker() {
211+
if (this.deprecated && this._taskRunProcesses.size === 0) {
212+
logger.debug("Worker deprecated, stopping", { outputPath: this.build.outputPath });
226213

227-
// Delete worker files
228-
this._onClose.post();
214+
this.params.stop();
215+
}
229216
}
230217

231218
get inProgressRuns(): Array<string> {
@@ -301,8 +288,6 @@ export class BackgroundWorker {
301288
throw new Error("Worker not initialized");
302289
}
303290

304-
this._closed = false;
305-
306291
logger.debug(this.#prefixedMessage(payload, "killing current task run process before attempt"));
307292

308293
await this.#killCurrentTaskRunProcessBeforeAttempt(payload.execution.run.id);
@@ -332,6 +317,8 @@ export class BackgroundWorker {
332317
// Only delete the task run process if the pid matches
333318
if (taskRunProcess?.pid === pid) {
334319
this._taskRunProcesses.delete(payload.execution.run.id);
320+
321+
this.#tryStopWorker();
335322
}
336323

337324
if (pid) {
@@ -435,10 +422,6 @@ export class BackgroundWorker {
435422
payload: TaskRunExecutionPayload,
436423
messageId: string
437424
): Promise<TaskRunExecutionResult> {
438-
if (this._closed) {
439-
throw new Error("Worker is closed");
440-
}
441-
442425
if (!this.manifest) {
443426
throw new Error("Worker not initialized");
444427
}

packages/cli-v3/src/dev/devSession.ts

+16-4
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ import { type DevCommandOptions } from "../commands/dev.js";
2323
import { eventBus } from "../utilities/eventBus.js";
2424
import { logger } from "../utilities/logger.js";
2525
import { resolveFileSources } from "../utilities/sourceFiles.js";
26-
import { EphemeralDirectory, getTmpDir } from "../utilities/tempDirectories.js";
26+
import { clearTmpDirs, EphemeralDirectory, getTmpDir } from "../utilities/tempDirectories.js";
2727
import { VERSION } from "../version.js";
2828
import { startDevOutput } from "./devOutput.js";
2929
import { startWorkerRuntime } from "./workerRuntime.js";
30+
import { existsSync, mkdirSync, rmSync } from "node:fs";
3031

3132
export type DevSessionOptions = {
3233
name: string | undefined;
@@ -37,6 +38,7 @@ export type DevSessionOptions = {
3738
rawArgs: DevCommandOptions;
3839
client: CliApiClient;
3940
onErr?: (error: Error) => void;
41+
keepTmpFiles: boolean;
4042
};
4143

4244
export type DevSessionInstance = {
@@ -49,8 +51,10 @@ export async function startDevSession({
4951
rawArgs,
5052
client,
5153
dashboardUrl,
54+
keepTmpFiles,
5255
}: DevSessionOptions): Promise<DevSessionInstance> {
53-
const destination = getTmpDir(rawConfig.workingDir, "build");
56+
clearTmpDirs(rawConfig.workingDir);
57+
const destination = getTmpDir(rawConfig.workingDir, "build", keepTmpFiles);
5458

5559
const runtime = await startWorkerRuntime({
5660
name,
@@ -96,7 +100,7 @@ export async function startDevSession({
96100
try {
97101
logger.debug("Updated bundle", { bundle, buildManifest });
98102

99-
await runtime.initializeWorker(buildManifest);
103+
await runtime.initializeWorker(buildManifest, workerDir?.remove ?? (() => {}));
100104
} catch (error) {
101105
if (error instanceof Error) {
102106
eventBus.emit("backgroundWorkerIndexingError", buildManifest, error);
@@ -124,6 +128,14 @@ export async function startDevSession({
124128
if (bundled) {
125129
eventBus.emit("rebuildStarted", "dev");
126130
}
131+
132+
const outdir = b.initialOptions.outdir;
133+
if (outdir && existsSync(outdir)) {
134+
logger.debug("Removing outdir", { outdir });
135+
136+
rmSync(outdir, { recursive: true, force: true });
137+
mkdirSync(outdir, { recursive: true });
138+
}
127139
});
128140
b.onEnd(async (result: esbuild.BuildResult) => {
129141
const errors = result.errors;
@@ -141,7 +153,7 @@ export async function startDevSession({
141153
// First bundle, no need to update bundle
142154
bundled = true;
143155
} else {
144-
const workerDir = getTmpDir(rawConfig.workingDir, "build");
156+
const workerDir = getTmpDir(rawConfig.workingDir, "build", keepTmpFiles);
145157

146158
await updateBuild(result, workerDir);
147159
}

packages/cli-v3/src/dev/workerRuntime.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { VERSION } from "../version.js";
3030

3131
export interface WorkerRuntime {
3232
shutdown(): Promise<void>;
33-
initializeWorker(manifest: BuildManifest): Promise<void>;
33+
initializeWorker(manifest: BuildManifest, stop: () => void): Promise<void>;
3434
}
3535

3636
export type WorkerRuntimeOptions = {
@@ -167,9 +167,10 @@ class DevWorkerRuntime implements WorkerRuntime {
167167
}
168168
}
169169

170-
async initializeWorker(manifest: BuildManifest, options?: { cwd?: string }): Promise<void> {
170+
async initializeWorker(manifest: BuildManifest, stop: () => void): Promise<void> {
171171
if (this.lastBuild && this.lastBuild.contentHash === manifest.contentHash) {
172172
eventBus.emit("workerSkipped");
173+
stop();
173174
return;
174175
}
175176

@@ -178,18 +179,21 @@ class DevWorkerRuntime implements WorkerRuntime {
178179
const backgroundWorker = new BackgroundWorker(manifest, {
179180
env,
180181
cwd: this.options.config.workingDir,
182+
stop,
181183
});
182184

183185
await backgroundWorker.initialize();
184186

185187
if (!backgroundWorker.manifest) {
188+
stop();
186189
throw new Error("Could not initialize worker");
187190
}
188191

189192
const issues = validateWorkerManifest(backgroundWorker.manifest);
190193

191194
if (issues.length > 0) {
192195
issues.forEach((issue) => logger.error(issue));
196+
stop();
193197
return;
194198
}
195199

@@ -213,6 +217,7 @@ class DevWorkerRuntime implements WorkerRuntime {
213217
);
214218

215219
if (!backgroundWorkerRecord.success) {
220+
stop();
216221
throw new Error(backgroundWorkerRecord.error);
217222
}
218223

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

+19-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,14 @@ export function getTmpDir(
3030
const tmpPrefix = path.join(tmpRoot, `${prefix}-`);
3131
const tmpDir = fs.realpathSync(fs.mkdtempSync(tmpPrefix));
3232

33-
let removeDir = keep ? () => {} : () => fs.rmSync(tmpDir, { recursive: true, force: true });
34-
let removeExitListener = keep ? () => {} : onExit(removeDir);
33+
const removeDir = () => {
34+
try {
35+
return fs.rmSync(tmpDir, { recursive: true, force: true });
36+
} catch (e) {
37+
// This sometimes fails on Windows with EBUSY
38+
}
39+
};
40+
const removeExitListener = keep ? () => {} : onExit(removeDir);
3541

3642
return {
3743
path: tmpDir,
@@ -41,3 +47,14 @@ export function getTmpDir(
4147
},
4248
};
4349
}
50+
51+
export function clearTmpDirs(projectRoot: string | undefined) {
52+
projectRoot ??= process.cwd();
53+
const tmpRoot = path.join(projectRoot, ".trigger", "tmp");
54+
55+
try {
56+
fs.rmSync(tmpRoot, { recursive: true, force: true });
57+
} catch (e) {
58+
// This sometimes fails on Windows with EBUSY
59+
}
60+
}

0 commit comments

Comments
 (0)