Improved cron syntax

[mishankov]

Summary by CodeRabbit

• New Features

  • Cron-based task scheduling with cron expressions and interval formats
  • Health check handler exposing app health as JSON
  • Methods to register startup tasks and domain repositories

• Documentation

  • Expanded scheduler docs with cron syntax guide and examples
  • Demo app showcasing cron-based schedulers

• Refactor

  • Improved health tracking with per-service timestamps and error details

• Tests

  • Scheduler tests updated for cron expression validation and cancellation handling

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

 _____________________________
< My other GPU is a Blackwell >
 -----------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

_{✏️ Tip: You can disable in-progress messages and the fortune message in your review settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

✅ Unit Test PR creation complete.

• Create PR with unit tests
• Commit unit tests in branch claude/issue-29-plan-QL6EI
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coveralls]

Pull Request Test Coverage Report for Build 21916308076

Details

• 33 of 52 (63.46%) changed or added relevant lines in 4 files are covered.
• 3 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.3%) to 38.444%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
application/healthcheck.go	0	1	0.0%
application/application.go	0	2	0.0%
application/health.go	0	7	0.0%
scheduler/scheduler.go	33	42	78.57%

Files with Coverage Reduction	New Missed Lines	%
application/health.go	1	0.0%
scheduler/scheduler.go	2	75.56%

Totals
Change from base Build 21670678201:	0.3%
Covered Lines:	509
Relevant Lines:	1324

---

💛 - Coveralls

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

application/healthcheck.go (1)

31-34: ⚠️ Potential issue | 🟡 Minor

Fix log message: "decode" should be "encode".

The error message says "failed to decode response to json" but the operation is encoding (marshaling to JSON), not decoding.

📝 Proposed fix

 	err := json.NewEncoder(w).Encode(health)
 	if err != nil {
-		log.ErrorContext(r.Context(), "failed to decode response to json", "error", err)
+		log.ErrorContext(r.Context(), "failed to encode response to json", "error", err)
 	}

application/health.go (1)

52-64: ⚠️ Potential issue | 🟠 Major

Potential nil pointer dereference on err parameter.

If FailService is called with a nil error, line 60 will panic when calling err.Error().

🛡️ Proposed fix to add nil check

 func (h *Health) FailService(serviceName string, err error) {
 	if service, ok := h.Services[serviceName]; ok {
 		service.Status = ServiceStatusError
 
 		st := time.Now()
 		service.StoppedAt = &st
 
-		service.Error = err.Error()
+		if err != nil {
+			service.Error = err.Error()
+		}
 
 		h.Services[serviceName] = service
 	}
 }

🤖 Fix all issues with AI agents

In `@application/health.go`:
- Around line 35-38: Update the doc comment for NewHealth to reference the
correct return type: replace "ApplicationHealth" with "Health" (e.g., "NewHealth
creates a Health with initialized storage.") so the comment matches the function
signature NewHealth() *Health; ensure ServiceHealth remains referenced correctly
if needed.

🧹 Nitpick comments (3)

scheduler/scheduler.go (1)

55-55: Consider caching the parser to avoid duplication.

The parser is created twice: once in New() for validation and again in Run(). While this is functionally correct, you could store the parser or the parsed schedule in the Scheduler struct to avoid recreating it.

♻️ Optional: Store parsed schedule in struct

 type Scheduler struct {
 	cronExpr string             // The cron expression
+	schedule cron.Schedule      // The parsed schedule
 	runner   application.Runner // The runner to execute periodically
 }

 func New(cronExpr string, runner application.Runner) (*Scheduler, error) {
 	if cronExpr == "" {
 		return nil, fmt.Errorf("invalid cron expression %q: %w", cronExpr, errEmptyCronExpression)
 	}

 	parser := cron.NewParser(cronParseOptions)

-	if _, err := parser.Parse(cronExpr); err != nil {
+	schedule, err := parser.Parse(cronExpr)
+	if err != nil {
 		return nil, fmt.Errorf("invalid cron expression %q: %w", cronExpr, err)
 	}

 	return &Scheduler{
 		cronExpr: cronExpr,
+		schedule: schedule,
 		runner:   runner,
 	}, nil
 }

Then in Run(), use the pre-parsed schedule directly with cronScheduler.Schedule(s.schedule, ...).

Also applies to: 71-71

demo-app/cmd/scheduler-cron/main.go (1)

85-96: Consider adding error observability for scheduler goroutines.

Errors returned from s.Run(ctx) are silently discarded. For a demo, this is acceptable, but you could improve observability by using an error channel or logging errors from each goroutine.

♻️ Optional: Add error logging in goroutines

 	// Start all schedulers in background
-	go s1.Run(ctx)
-	go s2.Run(ctx)
-	go s3.Run(ctx)
-	go s4.Run(ctx)
-	go s5.Run(ctx)
+	go func() {
+		if err := s1.Run(ctx); err != nil && err != context.Canceled {
+			log.ErrorContext(ctx, "scheduler 1 error", "error", err)
+		}
+	}()
+	go func() {
+		if err := s2.Run(ctx); err != nil && err != context.Canceled {
+			log.ErrorContext(ctx, "scheduler 2 error", "error", err)
+		}
+	}()
+	go func() {
+		if err := s3.Run(ctx); err != nil && err != context.Canceled {
+			log.ErrorContext(ctx, "scheduler 3 error", "error", err)
+		}
+	}()
+	go func() {
+		if err := s4.Run(ctx); err != nil && err != context.Canceled {
+			log.ErrorContext(ctx, "scheduler 4 error", "error", err)
+		}
+	}()
+	go func() {
+		if err := s5.Run(ctx); err != nil && err != context.Canceled {
+			log.ErrorContext(ctx, "scheduler 5 error", "error", err)
+		}
+	}()

application/application.go (1)

89-95: Consider adding nil check for domain parameter.

If domain is nil, calling domain.GetRepository() on line 92 will panic. A defensive nil check would make this API safer.

🛡️ Proposed defensive check

 // RegisterDomain registers a domain repository in the specified database.
 func (a *Application) RegisterDomain(name, dbName string, domain Domain) {
-	if dbName != "" {
+	if dbName != "" && domain != nil {
 		repository := domain.GetRepository()
 		a.RegisterRepository(dbName, name+"_repository", repository)
 	}
 }

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

application/health.go (2)

40-50: ⚠️ Potential issue | 🟡 Minor

Reset stale failure fields on restart.
With the new StoppedAt/Error fields, restarting a service should clear old failure info so status isn’t inconsistent.

Suggested fix

 func (h *Health) StartService(serviceName string) {
 	if service, ok := h.Services[serviceName]; ok {
 		service.Status = ServiceStatusStarted

 		st := time.Now()
 		service.StartedAt = &st
+		service.StoppedAt = nil
+		service.Error = ""

 		h.Services[serviceName] = service
 	}
 }

---

52-64: ⚠️ Potential issue | 🟡 Minor

Consider adding a nil guard for robustness.
While the current call site checks err != nil before invoking FailService, adding a defensive check inside the method would prevent potential panics if called from other contexts in the future.

🧹 Nitpick comments (1)

application/health.go (1)

74-77: Consider handling json.Marshal errors in String().
If Data includes non-marshable values, the current implementation silently returns an empty string. A small fallback avoids surprising results.

Suggested fix

 func (h *Health) String() string {
-	b, _ := json.Marshal(h)
-	return string(b)
+	b, err := json.Marshal(h)
+	if err != nil {
+		return "{}"
+	}
+	return string(b)
 }

[coderabbitai]

✅ Created PR with unit tests: #58