fix(scenario-runner): harden run failure handling across all paths
- add onModuleInit to fail in_progress runs on server restart
(playwright sessions cannot be recovered after restart)
- extract failRun() shared method used by restart cleanup, timeout,
and unexpected error paths for consistent step run status updates
- fix catch path to also fail/cancel step runs (previously left orphaned)
- accept plain {name, steps} payload in importScenario alongside
full export format; throw BadRequestException for invalid input
- include stepRuns relation in findRuns paginated response
- add scenario-scheduler.spec.ts integration tests for onModuleInit
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { Injectable } from "@nestjs/common";
|
||||
import { Injectable, OnModuleInit } from "@nestjs/common";
|
||||
import { Interval } from "@nestjs/schedule";
|
||||
import { InjectRepository } from "@nestjs/typeorm";
|
||||
import * as crypto from "crypto";
|
||||
@@ -30,7 +30,7 @@ interface BrowserHandle {
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class ScenarioSchedulerService {
|
||||
export class ScenarioSchedulerService implements OnModuleInit {
|
||||
private readonly logger = new TraceLogger(ScenarioSchedulerService.name);
|
||||
private readonly activeRuns = new Set<string>();
|
||||
private readonly runBrowsers = new Map<string, BrowserHandle>();
|
||||
@@ -62,6 +62,52 @@ export class ScenarioSchedulerService {
|
||||
private readonly sessionContextService: SessionContextService,
|
||||
) {}
|
||||
|
||||
async onModuleInit(): Promise<void> {
|
||||
const staleRuns = await this.runRepo.find({
|
||||
where: { status: "in_progress" },
|
||||
});
|
||||
if (staleRuns.length === 0) return;
|
||||
|
||||
this.logger.error(
|
||||
`Server restart detected ${staleRuns.length} in-progress run(s) that cannot be recovered — marking as failed`,
|
||||
);
|
||||
|
||||
for (const run of staleRuns) {
|
||||
this.logger.error(
|
||||
`Run #${run.id}: marked fail due to server restart (playwright session lost)`,
|
||||
);
|
||||
await this.failRun(
|
||||
run.id,
|
||||
"Run aborted: server restarted and playwright session could not be recovered",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks a run as failed, fails its active step run, cancels any queued
|
||||
* step runs, and persists an error log entry. Use this for all failure paths.
|
||||
*/
|
||||
private async failRun(runId: string, reason: string): Promise<void> {
|
||||
await this.runRepo.update(runId, { status: "fail" });
|
||||
// Fail the actively-running step; cancel anything still queued
|
||||
await this.runStepRepo
|
||||
.createQueryBuilder()
|
||||
.update()
|
||||
.set({ status: "fail" })
|
||||
.where("runId = :runId AND status = 'in_progress'", { runId })
|
||||
.execute();
|
||||
await this.runStepRepo
|
||||
.createQueryBuilder()
|
||||
.update()
|
||||
.set({ status: "cancelled" })
|
||||
.where("runId = :runId AND status IN (:...statuses)", {
|
||||
runId,
|
||||
statuses: ["pending", "waiting"],
|
||||
})
|
||||
.execute();
|
||||
this.persistLog(runId, null, "error", reason);
|
||||
}
|
||||
|
||||
private persistLog(
|
||||
runId: string,
|
||||
stepRunId: string | null,
|
||||
@@ -153,22 +199,10 @@ export class ScenarioSchedulerService {
|
||||
this.logger.error(
|
||||
`Run #${runId}: exceeded scenario timeout of ${scenarioTimeoutSec}s`,
|
||||
);
|
||||
this.persistLog(
|
||||
await this.failRun(
|
||||
runId,
|
||||
null,
|
||||
"error",
|
||||
`Scenario timed out after ${scenarioTimeoutSec}s`,
|
||||
);
|
||||
await this.runRepo.update(runId, { status: "fail" });
|
||||
await this.runStepRepo
|
||||
.createQueryBuilder()
|
||||
.update()
|
||||
.set({ status: "cancelled" })
|
||||
.where("runId = :runId AND status IN (:...statuses)", {
|
||||
runId,
|
||||
statuses: ["waiting", "pending", "in_progress"],
|
||||
})
|
||||
.execute();
|
||||
return;
|
||||
}
|
||||
await this.executeStepRun(stepRun);
|
||||
@@ -188,7 +222,7 @@ export class ScenarioSchedulerService {
|
||||
this.logger.error(
|
||||
`Run #${runId}: unexpected error: ${(err as Error).message}`,
|
||||
);
|
||||
await this.runRepo.update(runId, { status: "fail" });
|
||||
await this.failRun(runId, `Unexpected error: ${(err as Error).message}`);
|
||||
} finally {
|
||||
this.activeRuns.delete(runId);
|
||||
this.runCredentials.delete(runId);
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import {
|
||||
BadRequestException,
|
||||
ConflictException,
|
||||
Injectable,
|
||||
NotFoundException,
|
||||
@@ -302,6 +303,7 @@ export class ScenarioService {
|
||||
if (query.status) where["status"] = query.status;
|
||||
const [data, total] = await this.runRepo.findAndCount({
|
||||
where,
|
||||
relations: ["stepRuns"],
|
||||
order: { [orderBy]: orderDir },
|
||||
skip: (page - 1) * limit,
|
||||
take: limit,
|
||||
@@ -492,6 +494,18 @@ export class ScenarioService {
|
||||
async importScenario(
|
||||
payload: ScenarioExportDto | ExportEntity[],
|
||||
): Promise<ScenarioEntity> {
|
||||
// Accept plain {name, steps} objects (no kind) as well as the full export format
|
||||
const raw = payload as unknown as Record<string, unknown>;
|
||||
if (!Array.isArray(payload) && !raw["kind"]) {
|
||||
if (typeof raw["name"] !== "string" || !raw["name"]) {
|
||||
throw new BadRequestException("name is required");
|
||||
}
|
||||
if (!Array.isArray(raw["steps"])) {
|
||||
throw new BadRequestException("steps must be an array");
|
||||
}
|
||||
(payload as unknown as Record<string, unknown>)["kind"] = "scenario";
|
||||
}
|
||||
|
||||
const items: ExportEntity[] = Array.isArray(payload) ? payload : [payload];
|
||||
|
||||
let scenarioDto: ScenarioExportDto | undefined;
|
||||
@@ -535,13 +549,19 @@ export class ScenarioService {
|
||||
}),
|
||||
);
|
||||
}
|
||||
} else if (item.kind === "scenario") {
|
||||
} else if (!item.kind || item.kind === "scenario") {
|
||||
scenarioDto = item as ScenarioExportDto;
|
||||
}
|
||||
}
|
||||
|
||||
if (!scenarioDto) {
|
||||
throw new Error("No scenario entity found in import payload");
|
||||
throw new BadRequestException("No scenario entity found in import payload");
|
||||
}
|
||||
if (typeof scenarioDto.name !== "string" || !scenarioDto.name) {
|
||||
throw new BadRequestException("name is required");
|
||||
}
|
||||
if (!Array.isArray(scenarioDto.steps)) {
|
||||
throw new BadRequestException("steps must be an array");
|
||||
}
|
||||
|
||||
const dto = scenarioDto;
|
||||
|
||||
Reference in New Issue
Block a user