From e839f4241261ecacf68bd3e1f71c99fff0ed9828 Mon Sep 17 00:00:00 2001 From: viettranx Date: Fri, 10 Apr 2026 19:52:43 +0700 Subject: [PATCH] fix(cron): improve panic recovery and config validation - Use safego.Recover for per-job panic recovery in PG and SQLite cron schedulers (captures stack traces vs inline fmt.Sprint) - Log warning when cron job_timeout config value is invalid instead of silently falling back to default --- internal/config/config.go | 5 ++++- internal/store/pg/cron_scheduler.go | 10 +++------- internal/store/sqlitestore/cron_scheduler.go | 10 +++------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index dac412e4..b1639d41 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "fmt" + "log/slog" "sync" "time" @@ -352,9 +353,11 @@ const DefaultJobTimeout = 10 * time.Minute // JobTimeoutDuration returns the configured job timeout or the default (10m). func (cc CronConfig) JobTimeoutDuration() time.Duration { if cc.JobTimeout != "" { - if d, err := time.ParseDuration(cc.JobTimeout); err == nil && d > 0 { + d, err := time.ParseDuration(cc.JobTimeout) + if err == nil && d > 0 { return d } + slog.Warn("cron: invalid job_timeout, using default", "value", cc.JobTimeout, "default", DefaultJobTimeout) } return DefaultJobTimeout } diff --git a/internal/store/pg/cron_scheduler.go b/internal/store/pg/cron_scheduler.go index 8fcd3850..e1c6e388 100644 --- a/internal/store/pg/cron_scheduler.go +++ b/internal/store/pg/cron_scheduler.go @@ -10,6 +10,7 @@ import ( "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/cron" + "github.com/nextlevelbuilder/goclaw/internal/safego" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -195,13 +196,8 @@ func (s *PGCronStore) checkAndRunDueJobs() { // due jobs. Now each job runs independently; cache is invalidated per-job. for _, job := range claimedJobs { go func(job store.CronJob) { - defer func() { - if r := recover(); r != nil { - slog.Error("cron: job execution panicked", "job_id", job.ID, "job_name", job.Name, "panic", fmt.Sprint(r)) - } - // Invalidate cache so the next tick picks up the updated next_run_at. - s.InvalidateCache() - }() + defer safego.Recover(nil, "component", "cron_job", "job_id", job.ID, "job_name", job.Name) + defer s.InvalidateCache() s.executeOneJob(job, handler, true) }(job) } diff --git a/internal/store/sqlitestore/cron_scheduler.go b/internal/store/sqlitestore/cron_scheduler.go index b7518384..26302358 100644 --- a/internal/store/sqlitestore/cron_scheduler.go +++ b/internal/store/sqlitestore/cron_scheduler.go @@ -12,6 +12,7 @@ import ( "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/cron" + "github.com/nextlevelbuilder/goclaw/internal/safego" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -201,13 +202,8 @@ func (s *SQLiteCronStore) checkAndRunDueJobs() { // due jobs. Now each job runs independently; cache is invalidated per-job. for _, job := range claimedJobs { go func(job store.CronJob) { - defer func() { - if r := recover(); r != nil { - slog.Error("cron: job execution panicked", "job_id", job.ID, "job_name", job.Name, "panic", fmt.Sprint(r)) - } - // Invalidate cache so the next tick picks up the updated next_run_at. - s.InvalidateCache() - }() + defer safego.Recover(nil, "component", "cron_job", "job_id", job.ID, "job_name", job.Name) + defer s.InvalidateCache() s.executeOneJob(job, handler, true) }(job) }