Return an error rather than panicking on invalid cron specs

Author: robfig

constantdelay.go

@@ -9,12 +9,11 @@ type ConstantDelaySchedule struct {
 }
 
 // Every returns a crontab Schedule that activates once every duration.
-// Delays of less than a second are not supported (will panic).
+// Delays of less than a second are not supported (will round up to 1 second).
 // Any fields less than a Second are truncated.
 func Every(duration time.Duration) ConstantDelaySchedule {
 	if duration < time.Second {
-		panic("cron/constantdelay: delays of less than a second are not supported: " +
-			duration.String())
+		duration = time.Second
 	}
 	return ConstantDelaySchedule{
 		Delay: duration - time.Duration(duration.Nanoseconds())%time.Second,

constantdelay_test.go

@@ -34,6 +34,9 @@ func TestConstantDelayNext(t *testing.T) {
 		// Round to nearest second on the delay
 		{"Mon Jul 9 14:45 2012", 15*time.Minute + 50*time.Nanosecond, "Mon Jul 9 15:00 2012"},
 
+		// Round up to 1 second if the duration is less.
+		{"Mon Jul 9 14:45:00 2012", 15 * time.Millisecond, "Mon Jul 9 14:45:01 2012"},
+
 		// Round to nearest second when calculating the next time.
 		{"Mon Jul 9 14:45:00.005 2012", 15 * time.Minute, "Mon Jul 9 15:00 2012"},
 

cron.go

@@ -83,13 +83,18 @@ type FuncJob func()
 func (f FuncJob) Run() { f() }
 
 // AddFunc adds a func to the Cron to be run on the given schedule.
-func (c *Cron) AddFunc(spec string, cmd func()) {
-	c.AddJob(spec, FuncJob(cmd))
+func (c *Cron) AddFunc(spec string, cmd func()) error {
+	return c.AddJob(spec, FuncJob(cmd))
 }
 
 // AddFunc adds a Job to the Cron to be run on the given schedule.
-func (c *Cron) AddJob(spec string, cmd Job) {
-	c.Schedule(Parse(spec), cmd)
+func (c *Cron) AddJob(spec string, cmd Job) error {
+	schedule, err := Parse(spec)
+	if err != nil {
+		return err
+	}
+	c.Schedule(schedule, cmd)
+	return nil
 }
 
 // Schedule adds a Job to the Cron to be run on the given schedule.

parser.go

@@ -1,6 +1,7 @@
 package cron
 
 import (
+	"fmt"
 	"log"
 	"math"
 	"strconv"
@@ -9,14 +10,21 @@ import (
 )
 
 // Parse returns a new crontab schedule representing the given spec.
-// It panics with a descriptive error if the spec is not valid.
+// It returns a descriptive error if the spec is not valid.
 //
 // It accepts
 //   - Full crontab specs, e.g. "* * * * * ?"
 //   - Descriptors, e.g. "@midnight", "@every 1h30m"
-func Parse(spec string) Schedule {
+func Parse(spec string) (_ Schedule, err error) {
+	// Convert panics into errors
+	defer func() {
+		if recovered := recover(); recovered != nil {
+			err = fmt.Errorf("%v", recovered)
+		}
+	}()
+
 	if spec[0] == '@' {
-		return parseDescriptor(spec)
+		return parseDescriptor(spec), nil
 	}
 
 	// Split on whitespace.  We require 5 or 6 fields.
@@ -40,7 +48,7 @@ func Parse(spec string) Schedule {
 		Dow:    getField(fields[5], dow),
 	}
 
-	return schedule
+	return schedule, nil
 }
 
 // getField returns an Int with the bits set representing all of the times that

parser_test.go

@@ -106,7 +106,10 @@ func TestSpecSchedule(t *testing.T) {
 	}
 
 	for _, c := range entries {
-		actual := Parse(c.expr)
+		actual, err := Parse(c.expr)
+		if err != nil {
+			t.Error(err)
+		}
 		if !reflect.DeepEqual(actual, c.expected) {
 			t.Errorf("%s => (expected) %b != %b (actual)", c.expr, c.expected, actual)
 		}

spec_test.go

@@ -56,7 +56,12 @@ func TestActivation(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		actual := Parse(test.spec).Next(getTime(test.time).Add(-1 * time.Second))
+		sched, err := Parse(test.spec)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		actual := sched.Next(getTime(test.time).Add(-1 * time.Second))
 		expected := getTime(test.time)
 		if test.expected && expected != actual || !test.expected && expected == actual {
 			t.Errorf("Fail evaluating %s on %s: (expected) %s != %s (actual)",
@@ -117,14 +122,34 @@ func TestNext(t *testing.T) {
 	}
 
 	for _, c := range runs {
-		actual := Parse(c.spec).Next(getTime(c.time))
+		sched, err := Parse(c.spec)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		actual := sched.Next(getTime(c.time))
 		expected := getTime(c.expected)
 		if !actual.Equal(expected) {
 			t.Errorf("%s, \"%s\": (expected) %v != %v (actual)", c.time, c.spec, expected, actual)
 		}
 	}
 }
 
+func TestErrors(t *testing.T) {
+	invalidSpecs := []string{
+		"xyz",
+		"60 0 * * *",
+		"0 60 * * *",
+		"0 0 * * XYZ",
+	}
+	for _, spec := range invalidSpecs {
+		_, err := Parse(spec)
+		if err == nil {
+			t.Error("expected an error parsing: ", spec)
+		}
+	}
+}
+
 func getTime(value string) time.Time {
 	if value == "" {
 		return time.Time{}