Skip to content

Commit bc42123

Browse files
Copilotvcastellm
andcommitted
Address code review feedback: improve error handling and boundary conditions
Co-authored-by: vcastellm <47026+vcastellm@users.noreply.github.com>
1 parent 445f142 commit bc42123

File tree

3 files changed

+33
-8
lines changed

3 files changed

+33
-8
lines changed

extcron/after.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ func (schedule AfterSchedule) Next(t time.Time) time.Time {
3030
return schedule.Date
3131
}
3232

33-
// If we're within the grace period, run immediately
33+
// If we're within the grace period (including the exact end moment), run immediately
3434
gracePeriodEnd := schedule.Date.Add(schedule.GracePeriod)
35-
if t.Before(gracePeriodEnd) {
35+
if !t.After(gracePeriodEnd) {
3636
return t
3737
}
3838

extcron/after_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ func TestAfterScheduleNext(t *testing.T) {
3636
currentTime: "2020-01-01T01:59:59Z",
3737
expected: "2020-01-01T01:59:59Z", // Returns current time (immediate)
3838
},
39+
{
40+
name: "exactly at end of grace period - should run immediately",
41+
scheduleAt: "2020-01-01T00:00:00Z",
42+
gracePeriod: 2 * time.Hour,
43+
currentTime: "2020-01-01T02:00:00Z",
44+
expected: "2020-01-01T02:00:00Z", // Returns current time (immediate)
45+
},
3946
{
4047
name: "after grace period - should never run",
4148
scheduleAt: "2020-01-01T00:00:00Z",

extcron/duration.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,37 +27,55 @@ func ParseISO8601Duration(s string) (time.Duration, error) {
2727

2828
// Years (approximate: 365 days)
2929
if matches[1] != "" {
30-
years, _ := strconv.Atoi(matches[1])
30+
years, err := strconv.Atoi(matches[1])
31+
if err != nil {
32+
return 0, fmt.Errorf("invalid year value: %s", matches[1])
33+
}
3134
duration += time.Duration(years) * 365 * 24 * time.Hour
3235
}
3336

3437
// Months (approximate: 30 days)
3538
if matches[2] != "" {
36-
months, _ := strconv.Atoi(matches[2])
39+
months, err := strconv.Atoi(matches[2])
40+
if err != nil {
41+
return 0, fmt.Errorf("invalid month value: %s", matches[2])
42+
}
3743
duration += time.Duration(months) * 30 * 24 * time.Hour
3844
}
3945

4046
// Days
4147
if matches[3] != "" {
42-
days, _ := strconv.Atoi(matches[3])
48+
days, err := strconv.Atoi(matches[3])
49+
if err != nil {
50+
return 0, fmt.Errorf("invalid day value: %s", matches[3])
51+
}
4352
duration += time.Duration(days) * 24 * time.Hour
4453
}
4554

4655
// Hours
4756
if matches[4] != "" {
48-
hours, _ := strconv.Atoi(matches[4])
57+
hours, err := strconv.Atoi(matches[4])
58+
if err != nil {
59+
return 0, fmt.Errorf("invalid hour value: %s", matches[4])
60+
}
4961
duration += time.Duration(hours) * time.Hour
5062
}
5163

5264
// Minutes
5365
if matches[5] != "" {
54-
minutes, _ := strconv.Atoi(matches[5])
66+
minutes, err := strconv.Atoi(matches[5])
67+
if err != nil {
68+
return 0, fmt.Errorf("invalid minute value: %s", matches[5])
69+
}
5570
duration += time.Duration(minutes) * time.Minute
5671
}
5772

5873
// Seconds (can be fractional)
5974
if matches[6] != "" {
60-
seconds, _ := strconv.ParseFloat(matches[6], 64)
75+
seconds, err := strconv.ParseFloat(matches[6], 64)
76+
if err != nil {
77+
return 0, fmt.Errorf("invalid second value: %s", matches[6])
78+
}
6179
duration += time.Duration(seconds * float64(time.Second))
6280
}
6381

0 commit comments

Comments
 (0)