diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 6962041c1..07d80788a 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -235,7 +235,42 @@ func (cs *CronService) executeJobByID(jobID string) { } } else { nextRun := cs.computeNextRun(&job.Schedule, time.Now().UnixMilli()) - job.State.NextRunAtMS = nextRun + if nextRun != nil { + // Successfully calculated next run, update as usual + job.State.NextRunAtMS = nextRun + } else { + // Failed to compute next run - this indicates an issue with the schedule, prevent recurring->one-time conversion + log.Printf("[cron] Failed to compute next run for recurring job '%s' (%s), kind: %s", + job.Name, job.ID, job.Schedule.Kind) + // Handle differently based on schedule type to preserve recurrence + switch job.Schedule.Kind { + case "every": + // For every seconds jobs, compute fallback to prevent breaking recurrence due to temporary calculation issues + currentTime := time.Now().UnixMilli() + if job.Schedule.EveryMS != nil && *job.Schedule.EveryMS > 0 { + fallbackNextRun := currentTime + *job.Schedule.EveryMS + job.State.NextRunAtMS = &fallbackNextRun + } else { + // If interval is invalid, cannot schedule properly + job.State.NextRunAtMS = nil + job.Enabled = false // Also disable as invalid configuration + } + case "cron": + // For cron expression jobs, check if expression is invalid and disable appropriately + if job.Schedule.Expr == "" { + log.Printf("[cron] Empty cron expression for job '%s' (%s), disabling job", job.Name, job.ID) + job.Enabled = false + } else { + // Try to validate the expression further and potentially disable if repeatedly invalid + log.Printf("[cron] Invalid cron expression '%s' for job '%s' (%s), disabling job", + job.Schedule.Expr, job.Name, job.ID) + job.Enabled = false // Disable to prevent continuous errors from malformed cron expression + } + default: + // For other unrecognized schedule types, preserve as nil + job.State.NextRunAtMS = nil + } + } } if err := cs.saveStoreUnsafe(); err != nil { diff --git a/pkg/cron/service_test.go b/pkg/cron/service_test.go index 1a0dd1829..a6228b10f 100644 --- a/pkg/cron/service_test.go +++ b/pkg/cron/service_test.go @@ -33,6 +33,88 @@ func TestSaveStore_FilePermissions(t *testing.T) { } } +// TestRecurringJobsPreserveSchedule verifies that recurring jobs maintain their schedule after execution. +func TestRecurringJobsPreserveSchedule(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "cron", "jobs.json") + + handler := func(job *CronJob) (string, error) { + return "done", nil + } + + cs := NewCronService(storePath, handler) + + // Add an 'every' type recurring job + everyMS := int64(1000) // 1 second intervals + job, err := cs.AddJob("recurring-test", CronSchedule{Kind: "every", EveryMS: &everyMS}, "hello recurring", false, "cli", "direct") + if err != nil { + t.Fatalf("AddJob failed: %v", err) + } + + // Initially, the job should be enabled and have a next run time + if !job.Enabled { + t.Errorf("Job should be enabled initially"); + } + if job.State.NextRunAtMS == nil { + t.Errorf("Job should have a next run time initially"); + } + + // Execute the job manually once + cs.executeJobByID(job.ID) + + // Verify the job still exists, is enabled, and has next run time + foundJob := false + for _, j := range cs.ListJobs(true) { // include disabled jobs + if j.ID == job.ID { + foundJob = true + // Check that it's still enabled and has NextRunAtMS + if !j.Enabled { + t.Errorf("Recurring 'every' job was disabled after execution - this indicates a bug where recurring jobs become one-time.") + } + if j.State.NextRunAtMS == nil { + t.Errorf("Recurring 'every' job lost its next run time after execution"); + } + break + } + } + + if !foundJob { + t.Errorf("Job not found after execution - it was incorrectly removed.") + } +} + +// TestInvalidCronExpressionIsHandledGracefully checks that cron jobs with invalid expressions are handled appropriately +func TestInvalidCronExpressionIsHandledGracefully(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "cron", "jobs.json") + + cs := NewCronService(storePath, func(job *CronJob) (string, error) { + return "ok", nil + }) + + // Add a cron job with invalid expression + job, err := cs.AddJob("invalid-cron-test", CronSchedule{Kind: "cron", Expr: "not-a-valid-cron-expr"}, "invalid cron", false, "cli", "direct") + if err != nil { + t.Fatalf("AddJob for invalid cron job failed: %v", err) + } + + // Execute the job - this should not crash and should handle gracefully + cs.executeJobByID(job.ID) + + // For invalid cron expressions, the job should possibly be disabled after execution based on our fix + var jobFound *CronJob + for _, j := range cs.ListJobs(true) { + if j.ID == job.ID { + jobFound = &j + break + } + } + + if jobFound == nil { + t.Fatalf("Job not found after execution") + } +} + func int64Ptr(v int64) *int64 { return &v }