-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(schedule): await cloud schedule registration and validate jobs service response (#5139) #5450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| vi.mock("@dokploy/server", () => ({ | ||
| IS_CLOUD: true, | ||
| removeScheduleBackup: vi.fn(), | ||
| })); | ||
|
|
||
| import { | ||
| cancelJobs, | ||
| removeJob, | ||
| schedule, | ||
| updateJob, | ||
| } from "../../server/utils/backup"; | ||
|
|
||
| describe("Cloud Schedule Job Registration & Error Propagation (#5139)", () => { | ||
| const originalEnv = process.env; | ||
|
|
||
| beforeEach(() => { | ||
| process.env = { | ||
| ...originalEnv, | ||
| JOBS_URL: "https://jobs.example.com", | ||
| API_KEY: "test-secret-key", | ||
| }; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.env = originalEnv; | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("should send correct payload and return response on successful schedule creation", async () => { | ||
| const mockResponseData = { success: true, jobId: "job-123" }; | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| status: 200, | ||
| json: async () => mockResponseData, | ||
| } as unknown as Response); | ||
|
|
||
| const jobPayload = { | ||
| scheduleId: "sched-123", | ||
| type: "schedule" as const, | ||
| cronSchedule: "*/5 * * * *", | ||
| timezone: "UTC", | ||
| }; | ||
|
|
||
| const result = await schedule(jobPayload); | ||
|
|
||
| expect(global.fetch).toHaveBeenCalledTimes(1); | ||
| expect(global.fetch).toHaveBeenCalledWith( | ||
| "https://jobs.example.com/create-backup", | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "X-API-Key": "test-secret-key", | ||
| }, | ||
| body: JSON.stringify(jobPayload), | ||
| }, | ||
| ); | ||
| expect(result).toEqual(mockResponseData); | ||
| }); | ||
|
|
||
| it("should throw a descriptive error when cloud schedule creation returns non-2xx status", async () => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 500, | ||
| statusText: "Internal Server Error", | ||
| text: async () => "Scheduler worker unavailable", | ||
| } as unknown as Response); | ||
|
|
||
| const jobPayload = { | ||
| scheduleId: "sched-500", | ||
| type: "schedule" as const, | ||
| cronSchedule: "* * * * *", | ||
| timezone: "UTC", | ||
| }; | ||
|
|
||
| await expect(schedule(jobPayload)).rejects.toThrow( | ||
| "Failed to create schedule job: Internal Server Error Scheduler worker unavailable", | ||
| ); | ||
| }); | ||
|
|
||
| it("should throw a descriptive error when updateJob returns non-2xx status", async () => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 400, | ||
| statusText: "Bad Request", | ||
| text: async () => "Invalid cron expression", | ||
| } as unknown as Response); | ||
|
|
||
| const jobPayload = { | ||
| scheduleId: "sched-400", | ||
| type: "schedule" as const, | ||
| cronSchedule: "invalid-cron", | ||
| timezone: "UTC", | ||
| }; | ||
|
|
||
| await expect(updateJob(jobPayload)).rejects.toThrow( | ||
| "Failed to update schedule job: Bad Request Invalid cron expression", | ||
| ); | ||
| }); | ||
|
|
||
| it("should throw a descriptive error when removeJob returns non-2xx status", async () => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 404, | ||
| statusText: "Not Found", | ||
| text: async () => "Job not registered", | ||
| } as unknown as Response); | ||
|
|
||
| const jobPayload = { | ||
| scheduleId: "sched-404", | ||
| type: "schedule" as const, | ||
| cronSchedule: "0 0 * * *", | ||
| timezone: "UTC", | ||
| }; | ||
|
|
||
| await expect(removeJob(jobPayload)).rejects.toThrow( | ||
| "Failed to remove schedule job: Not Found Job not registered", | ||
| ); | ||
| }); | ||
|
|
||
| it("should propagate network failure when fetch rejects", async () => { | ||
| global.fetch = vi | ||
| .fn() | ||
| .mockRejectedValue(new Error("Network connection refused")); | ||
|
|
||
| const jobPayload = { | ||
| scheduleId: "sched-net-err", | ||
| type: "schedule" as const, | ||
| cronSchedule: "*/10 * * * *", | ||
| timezone: "UTC", | ||
| }; | ||
|
|
||
| await expect(schedule(jobPayload)).rejects.toThrow( | ||
| "Network connection refused", | ||
| ); | ||
| }); | ||
|
|
||
| describe("cancelJobs", () => { | ||
| it("should remove all enabled backup jobs when in cloud mode", async () => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| status: 200, | ||
| json: async () => ({ success: true }), | ||
| } as unknown as Response); | ||
|
|
||
| const backups = [ | ||
| { backupId: "b1", schedule: "0 0 * * *", enabled: true }, | ||
| { backupId: "b2", schedule: "0 12 * * *", enabled: false }, | ||
| { backupId: "b3", schedule: "0 6 * * *", enabled: true }, | ||
| ]; | ||
|
|
||
| await cancelJobs(backups as any); | ||
|
|
||
| // b1 and b3 are enabled, b2 is disabled | ||
| expect(global.fetch).toHaveBeenCalledTimes(2); | ||
| expect(global.fetch).toHaveBeenCalledWith( | ||
| "https://jobs.example.com/remove-job", | ||
| expect.objectContaining({ | ||
| body: JSON.stringify({ | ||
| cronSchedule: "0 0 * * *", | ||
| backupId: "b1", | ||
| type: "backup", | ||
| }), | ||
| }), | ||
| ); | ||
| expect(global.fetch).toHaveBeenCalledWith( | ||
| "https://jobs.example.com/remove-job", | ||
| expect.objectContaining({ | ||
| body: JSON.stringify({ | ||
| cronSchedule: "0 6 * * *", | ||
| backupId: "b3", | ||
| type: "backup", | ||
| }), | ||
| }), | ||
| ); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import { | |
| type BackupScheduleList, | ||
| IS_CLOUD, | ||
| removeScheduleBackup, | ||
| } from "@dokploy/server/index"; | ||
| } from "@dokploy/server"; | ||
|
|
||
| type QueueJob = | ||
| | { | ||
|
|
@@ -36,6 +36,12 @@ export const schedule = async (job: QueueJob) => { | |
| }, | ||
| body: JSON.stringify(job), | ||
| }); | ||
| if (!result.ok) { | ||
| const errorText = await result.text().catch(() => ""); | ||
| throw new Error( | ||
| `Failed to create schedule job: ${result.statusText} ${errorText}`.trim(), | ||
| ); | ||
| } | ||
| const data = await result.json(); | ||
| return data; | ||
| } catch (error) { | ||
|
|
@@ -53,6 +59,12 @@ export const removeJob = async (job: QueueJob) => { | |
| }, | ||
| body: JSON.stringify(job), | ||
| }); | ||
| if (!result.ok) { | ||
| const errorText = await result.text().catch(() => ""); | ||
| throw new Error( | ||
| `Failed to remove schedule job: ${result.statusText} ${errorText}`.trim(), | ||
| ); | ||
|
Comment on lines
+62
to
+66
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Callers commit local updates or deletions before these new non-2xx errors are thrown. For example, schedule deletion removes the database row before awaiting |
||
| } | ||
| const data = await result.json(); | ||
| return data; | ||
| } catch (error) { | ||
|
|
@@ -70,6 +82,12 @@ export const updateJob = async (job: QueueJob) => { | |
| }, | ||
| body: JSON.stringify(job), | ||
| }); | ||
| if (!result.ok) { | ||
| const errorText = await result.text().catch(() => ""); | ||
| throw new Error( | ||
| `Failed to update schedule job: ${result.statusText} ${errorText}`.trim(), | ||
| ); | ||
| } | ||
| const data = await result.json(); | ||
| return data; | ||
| } catch (error) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When cloud registration fails, the enabled schedule has already been committed to the database, so this awaited call returns an error without removing that row. The dialog remains open and permits another submission, which inserts a schedule with a new ID. Repeated attempts can therefore create duplicate enabled schedules with no registered cloud job.