From e725128b10c040d0d9f5864b532c2a9fd953e076 Mon Sep 17 00:00:00 2001 From: harsh-aghara Date: Tue, 5 May 2026 17:08:25 +0530 Subject: [PATCH] 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 --- server/src/controllers/monitorController.ts | 9 ++- server/src/service/business/monitorService.ts | 29 ++++++++-- .../test/unit/services/monitorService.test.ts | 55 +++++++++++++++++-- 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/server/src/controllers/monitorController.ts b/server/src/controllers/monitorController.ts index 7422d5abb..5cb6f32be 100644 --- a/server/src/controllers/monitorController.ts +++ b/server/src/controllers/monitorController.ts @@ -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) { diff --git a/server/src/service/business/monitorService.ts b/server/src/service/business/monitorService.ts index e2430420d..d0dc8ca46 100644 --- a/server/src/service/business/monitorService.ts +++ b/server/src/service/business/monitorService.ts @@ -73,7 +73,7 @@ export interface IMonitorService { // update editMonitor(args: { teamId: string; monitorId: string; body: Partial }): Promise; pauseMonitor(args: { teamId: string; monitorId: string }): Promise; - bulkPauseMonitors(args: { teamId: string; monitorIds: string[]; pause: boolean }): Promise; + bulkPauseMonitors(args: { teamId: string; monitorIds: string[]; pause: boolean }): Promise<{ monitors: Monitor[]; failedCount: number }>; // delete deleteMonitor(args: { teamId: string; monitorId: string }): Promise; @@ -463,10 +463,18 @@ export class MonitorService implements IMonitorService { return monitor; }; - bulkPauseMonitors = async ({ teamId, monitorIds, pause }: { teamId: string; monitorIds: string[]; pause: boolean }): Promise => { + 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 => { diff --git a/server/test/unit/services/monitorService.test.ts b/server/test/unit/services/monitorService.test.ts index 74a0efa98..d2ee164bc 100644 --- a/server/test/unit/services/monitorService.test.ts +++ b/server/test/unit/services/monitorService.test.ts @@ -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", () => {