feat(monitors): track and report bulk action queue failures
Addresses PR feedback: - Upgrades bulk pausing from Promise.all to Promise.allSettled - Logs individual job queue sync failures with monitor ID context - Surfaces failedCount to the frontend via controller response - Adds unit tests to verify partial failure handling Fixes #3602
This commit is contained in:
1 parent
baf7344e2d
commit
e725128b10
3 files changed
+82
-11
No files matched your search
@@ -315,14 +315,19 @@ class MonitorController implements IMonitorController {
|
||||
const { monitorIds, pause } = validatedBody;
|
||||
const teamId = requireTeamId(req.user?.teamId);
|
||||
|
||||
const monitors = await this.monitorService.bulkPauseMonitors({ teamId, monitorIds, pause });
|
||||
const { monitors, failedCount } = await this.monitorService.bulkPauseMonitors({ teamId, monitorIds, pause });
|
||||
|
||||
const action = pause ? "paused" : "resumed";
|
||||
const monitorStr = monitors.length === 1 ? "monitor" : "monitors";
|
||||
|
||||
let msg = `${monitors.length} ${monitorStr} ${action} successfully`;
|
||||
if (failedCount > 0) {
|
||||
msg = `${monitors.length} ${monitorStr} ${action} in database, but ${failedCount} failed to sync with the job queue. Please check logs.`;
|
||||
}
|
||||
|
||||
return res.status(200).json({
|
||||
success: true,
|
||||
msg: `${monitors.length} ${monitorStr} ${action} successfully`,
|
||||
msg,
|
||||
data: monitors,
|
||||
});
|
||||
} catch (error) {
|
||||
|
||||
@@ -73,7 +73,7 @@ export interface IMonitorService {
|
||||
// update
|
||||
editMonitor(args: { teamId: string; monitorId: string; body: Partial<Monitor> }): Promise<Monitor>;
|
||||
pauseMonitor(args: { teamId: string; monitorId: string }): Promise<Monitor>;
|
||||
bulkPauseMonitors(args: { teamId: string; monitorIds: string[]; pause: boolean }): Promise<Monitor[]>;
|
||||
bulkPauseMonitors(args: { teamId: string; monitorIds: string[]; pause: boolean }): Promise<{ monitors: Monitor[]; failedCount: number }>;
|
||||
|
||||
// delete
|
||||
deleteMonitor(args: { teamId: string; monitorId: string }): Promise<Monitor>;
|
||||
@@ -463,10 +463,18 @@ export class MonitorService implements IMonitorService {
|
||||
return monitor;
|
||||
};
|
||||
|
||||
bulkPauseMonitors = async ({ teamId, monitorIds, pause }: { teamId: string; monitorIds: string[]; pause: boolean }): Promise<Monitor[]> => {
|
||||
bulkPauseMonitors = async ({
|
||||
teamId,
|
||||
monitorIds,
|
||||
pause,
|
||||
}: {
|
||||
teamId: string;
|
||||
monitorIds: string[];
|
||||
pause: boolean;
|
||||
}): Promise<{ monitors: Monitor[]; failedCount: number }> => {
|
||||
const monitors = await this.monitorsRepository.bulkTogglePause(monitorIds, teamId, pause);
|
||||
|
||||
await Promise.all(
|
||||
const results = await Promise.allSettled(
|
||||
monitors.map(async (monitor) => {
|
||||
if (monitor.isActive) {
|
||||
await this.jobQueue.resumeJob(monitor);
|
||||
@@ -476,7 +484,20 @@ export class MonitorService implements IMonitorService {
|
||||
})
|
||||
);
|
||||
|
||||
return monitors;
|
||||
let failedCount = 0;
|
||||
results.forEach((result, index) => {
|
||||
if (result.status === "rejected") {
|
||||
failedCount++;
|
||||
this.logger.error({
|
||||
message: `Failed to sync job queue for monitor ${monitors[index].id} during bulk ${pause ? "pause" : "resume"}`,
|
||||
service: SERVICE_NAME,
|
||||
method: "bulkPauseMonitors",
|
||||
stack: result.reason instanceof Error ? result.reason.stack : undefined,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
return { monitors, failedCount };
|
||||
};
|
||||
|
||||
deleteMonitor = async ({ teamId, monitorId }: { teamId: string; monitorId: string }): Promise<Monitor> => {
|
||||
|
||||
@@ -853,7 +853,8 @@ describe("MonitorService", () => {
|
||||
pause: false,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.monitors).toHaveLength(2);
|
||||
expect(result.failedCount).toBe(0);
|
||||
expect(monitorsRepository.bulkTogglePause).toHaveBeenCalledWith(["m1", "m2"], TEAM_ID, false);
|
||||
expect(jobQueue.resumeJob).toHaveBeenCalledTimes(2);
|
||||
expect(jobQueue.pauseJob).not.toHaveBeenCalled();
|
||||
@@ -871,7 +872,8 @@ describe("MonitorService", () => {
|
||||
pause: true,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.monitors).toHaveLength(2);
|
||||
expect(result.failedCount).toBe(0);
|
||||
expect(monitorsRepository.bulkTogglePause).toHaveBeenCalledWith(["m1", "m2"], TEAM_ID, true);
|
||||
expect(jobQueue.pauseJob).toHaveBeenCalledTimes(2);
|
||||
expect(jobQueue.resumeJob).not.toHaveBeenCalled();
|
||||
@@ -890,7 +892,8 @@ describe("MonitorService", () => {
|
||||
pause: true,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.monitors).toHaveLength(2);
|
||||
expect(result.failedCount).toBe(0);
|
||||
expect(jobQueue.resumeJob).toHaveBeenCalledTimes(1);
|
||||
expect(jobQueue.resumeJob).toHaveBeenCalledWith(activeMonitor);
|
||||
expect(jobQueue.pauseJob).toHaveBeenCalledTimes(1);
|
||||
@@ -908,7 +911,8 @@ describe("MonitorService", () => {
|
||||
pause: true,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(0);
|
||||
expect(result.monitors).toHaveLength(0);
|
||||
expect(result.failedCount).toBe(0);
|
||||
expect(jobQueue.pauseJob).not.toHaveBeenCalled();
|
||||
expect(jobQueue.resumeJob).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -925,9 +929,50 @@ describe("MonitorService", () => {
|
||||
pause: true,
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result.monitors).toHaveLength(1);
|
||||
expect(result.failedCount).toBe(0);
|
||||
expect(jobQueue.pauseJob).toHaveBeenCalledWith(monitor);
|
||||
});
|
||||
|
||||
it("returns failedCount and logs errors when multiple job operations reject", async () => {
|
||||
const monitorsRepository = createMonitorsRepositoryMock();
|
||||
const monitors = [
|
||||
makeMonitor({ id: "m1", isActive: false }),
|
||||
makeMonitor({ id: "m2", isActive: false }),
|
||||
makeMonitor({ id: "m3", isActive: false }),
|
||||
];
|
||||
(monitorsRepository.bulkTogglePause as jest.Mock).mockResolvedValue(monitors);
|
||||
const jobQueue = createJobQueueMock();
|
||||
(jobQueue.pauseJob as jest.Mock)
|
||||
.mockRejectedValueOnce(new Error("Queue failure 1"))
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockRejectedValueOnce(new Error("Queue failure 2"));
|
||||
|
||||
const logger = createMockLogger();
|
||||
const { service } = createService({ monitorsRepository, jobQueue, logger });
|
||||
|
||||
const result = await service.bulkPauseMonitors({
|
||||
teamId: TEAM_ID,
|
||||
monitorIds: ["m1", "m2", "m3"],
|
||||
pause: true,
|
||||
});
|
||||
|
||||
expect(result.monitors).toHaveLength(3);
|
||||
expect(result.failedCount).toBe(2);
|
||||
expect(logger.error).toHaveBeenCalledTimes(2);
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("Failed to sync job queue for monitor m1"),
|
||||
method: "bulkPauseMonitors",
|
||||
})
|
||||
);
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("Failed to sync job queue for monitor m3"),
|
||||
method: "bulkPauseMonitors",
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("deleteMonitor", () => {
|
||||
|
||||
Reference in new issue
Block a user