From 2a46b355dbbf718bd7d19ee1210b7f57bf26ee2e Mon Sep 17 00:00:00 2001 From: Andrii Arsenin Date: Fri, 17 Apr 2026 14:11:43 +0300 Subject: [PATCH] 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 --- .../scenario/scenario-scheduler.service.ts | 66 ++++-- server/src/scenario/scenario.service.ts | 24 ++- server/test/scenario-scheduler.spec.ts | 189 ++++++++++++++++++ 3 files changed, 261 insertions(+), 18 deletions(-) create mode 100644 server/test/scenario-scheduler.spec.ts diff --git a/server/src/scenario/scenario-scheduler.service.ts b/server/src/scenario/scenario-scheduler.service.ts index 47d7d51..4188c7f 100644 --- a/server/src/scenario/scenario-scheduler.service.ts +++ b/server/src/scenario/scenario-scheduler.service.ts @@ -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(); private readonly runBrowsers = new Map(); @@ -62,6 +62,52 @@ export class ScenarioSchedulerService { private readonly sessionContextService: SessionContextService, ) {} + async onModuleInit(): Promise { + 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 { + 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); diff --git a/server/src/scenario/scenario.service.ts b/server/src/scenario/scenario.service.ts index a261063..4896db7 100644 --- a/server/src/scenario/scenario.service.ts +++ b/server/src/scenario/scenario.service.ts @@ -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 { + // Accept plain {name, steps} objects (no kind) as well as the full export format + const raw = payload as unknown as Record; + 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)["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; diff --git a/server/test/scenario-scheduler.spec.ts b/server/test/scenario-scheduler.spec.ts new file mode 100644 index 0000000..6a83a8a --- /dev/null +++ b/server/test/scenario-scheduler.spec.ts @@ -0,0 +1,189 @@ +import { INestApplication } from "@nestjs/common"; +import { getRepositoryToken } from "@nestjs/typeorm"; +import { Repository } from "typeorm"; +import { buildTestApp } from "./app.harness"; +import { ScenarioSchedulerService } from "../src/scenario/scenario-scheduler.service"; +import { ScenarioRunEntity } from "../src/scenario/scenario-run.entity"; +import { ScenarioRunStepEntity } from "../src/scenario/scenario-run-step.entity"; +import { ScenarioRunLogEntity } from "../src/scenario/scenario-run-log.entity"; +import { ScenarioEntity } from "../src/scenario/scenario.entity"; +import { ScenarioStepEntity } from "../src/scenario/scenario-step.entity"; + +describe("ScenarioSchedulerService.onModuleInit", () => { + let app: INestApplication; + let scheduler: ScenarioSchedulerService; + let scenarioRepo: Repository; + let stepRepo: Repository; + let runRepo: Repository; + let runStepRepo: Repository; + let runLogRepo: Repository; + + beforeAll(async () => { + app = await buildTestApp(); + scheduler = app.get(ScenarioSchedulerService); + scenarioRepo = app.get>( + getRepositoryToken(ScenarioEntity), + ); + stepRepo = app.get>( + getRepositoryToken(ScenarioStepEntity), + ); + runRepo = app.get>( + getRepositoryToken(ScenarioRunEntity), + ); + runStepRepo = app.get>( + getRepositoryToken(ScenarioRunStepEntity), + ); + runLogRepo = app.get>( + getRepositoryToken(ScenarioRunLogEntity), + ); + }); + + afterAll(async () => { + await app.close(); + }); + + // ── helpers ──────────────────────────────────────────────────────────────── + + async function seedScenarioWithStep() { + const scenario = await scenarioRepo.save( + scenarioRepo.create({ name: "test-scenario" }), + ); + const step = await stepRepo.save( + stepRepo.create({ scenarioId: scenario.id, order: 1 }), + ); + return { scenario, step }; + } + + async function seedRun( + scenarioId: string, + runStatus: ScenarioRunEntity["status"], + ) { + return runRepo.save( + runRepo.create({ scenarioId, status: runStatus }), + ); + } + + async function seedStepRun( + runId: string, + scenarioStepId: string, + status: ScenarioRunStepEntity["status"], + ) { + return runStepRepo.save( + runStepRepo.create({ runId, scenarioStepId, order: 1, status }), + ); + } + + // ── tests ────────────────────────────────────────────────────────────────── + + it("does nothing when no in_progress runs exist", async () => { + // All existing runs (if any) should already be clean after app init. + // This call should be a no-op and not throw. + await expect(scheduler.onModuleInit()).resolves.toBeUndefined(); + }); + + it("fails an in_progress run left from a previous server instance", async () => { + const { scenario } = await seedScenarioWithStep(); + const run = await seedRun(scenario.id, "in_progress"); + + await scheduler.onModuleInit(); + + const updated = await runRepo.findOneByOrFail({ id: run.id }); + expect(updated.status).toBe("fail"); + }); + + it("leaves pending and completed runs untouched", async () => { + const { scenario } = await seedScenarioWithStep(); + const pending = await seedRun(scenario.id, "pending"); + const passed = await seedRun(scenario.id, "pass"); + const failed = await seedRun(scenario.id, "fail"); + + await scheduler.onModuleInit(); + + expect((await runRepo.findOneByOrFail({ id: pending.id })).status).toBe( + "pending", + ); + expect((await runRepo.findOneByOrFail({ id: passed.id })).status).toBe( + "pass", + ); + expect((await runRepo.findOneByOrFail({ id: failed.id })).status).toBe( + "fail", + ); + }); + + it("fails in_progress step runs belonging to the stale run", async () => { + const { scenario, step } = await seedScenarioWithStep(); + const run = await seedRun(scenario.id, "in_progress"); + const stepRun = await seedStepRun(run.id, step.id, "in_progress"); + + await scheduler.onModuleInit(); + + const updated = await runStepRepo.findOneByOrFail({ id: stepRun.id }); + expect(updated.status).toBe("fail"); + }); + + it("cancels pending and waiting step runs of a stale run", async () => { + const { scenario, step } = await seedScenarioWithStep(); + const run = await seedRun(scenario.id, "in_progress"); + const pendingStep = await seedStepRun(run.id, step.id, "pending"); + const waitingStep = await seedStepRun(run.id, step.id, "waiting"); + + await scheduler.onModuleInit(); + + expect( + (await runStepRepo.findOneByOrFail({ id: pendingStep.id })).status, + ).toBe("cancelled"); + expect( + (await runStepRepo.findOneByOrFail({ id: waitingStep.id })).status, + ).toBe("cancelled"); + }); + + it("does not affect step runs of non-stale runs", async () => { + const { scenario, step } = await seedScenarioWithStep(); + const staleRun = await seedRun(scenario.id, "in_progress"); + const passedRun = await seedRun(scenario.id, "pass"); + const staleStep = await seedStepRun(staleRun.id, step.id, "in_progress"); + const passedStep = await seedStepRun(passedRun.id, step.id, "pass"); + + await scheduler.onModuleInit(); + + // in_progress step of the stale run → fail + expect( + (await runStepRepo.findOneByOrFail({ id: staleStep.id })).status, + ).toBe("fail"); + // step of a completed run → untouched + expect( + (await runStepRepo.findOneByOrFail({ id: passedStep.id })).status, + ).toBe("pass"); + }); + + it("persists an error log entry for each failed run", async () => { + const { scenario } = await seedScenarioWithStep(); + const run = await seedRun(scenario.id, "in_progress"); + + await scheduler.onModuleInit(); + + // Allow the void log persist to flush (persistLog uses void save) + await new Promise((r) => setTimeout(r, 50)); + + const logs = await runLogRepo.findBy({ runId: run.id }); + expect(logs.length).toBeGreaterThanOrEqual(1); + const errorLog = logs.find((l) => l.level === "error"); + expect(errorLog).toBeDefined(); + expect(errorLog!.message).toMatch(/server restarted/i); + }); + + it("handles multiple stale runs in one pass", async () => { + const { scenario } = await seedScenarioWithStep(); + const run1 = await seedRun(scenario.id, "in_progress"); + const run2 = await seedRun(scenario.id, "in_progress"); + + await scheduler.onModuleInit(); + + expect((await runRepo.findOneByOrFail({ id: run1.id })).status).toBe( + "fail", + ); + expect((await runRepo.findOneByOrFail({ id: run2.id })).status).toBe( + "fail", + ); + }); +});