tests(middleware): recovery adds unit tests and removes the original Recover

cron.go

@@ -127,6 +127,11 @@ func New(opts ...Option) *Cron {
 	return c
 }
 
+// Use adds a middleware to the chain of all jobs.
+func (c *Cron) Use(middleware ...Middleware) {
+	c.middlewares = append(c.middlewares, middleware...)
+}
+
 // AddFunc adds a func to the Cron to be run on the given schedule.
 // The spec is parsed using the time zone of this Cron instance as the default.
 // An opaque ID is returned that can be used to later remove it.

cron_test.go

@@ -4,19 +4,21 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"log"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 // Many tests schedule a job for every second, and then wait at most a second
 // for it to run.  This amount is just slightly larger than 1 second to
 // compensate for a few milliseconds of runtime.
 const OneSecond = 1*time.Second + 50*time.Millisecond //nolint:revive
 
+// syncWriter is a threadsafe writer.
+// Deprecated: use logger.NewBufferLogger instead.
 type syncWriter struct {
 	wr bytes.Buffer
 	m  sync.Mutex
@@ -35,48 +37,6 @@ func (sw *syncWriter) String() string {
 	return sw.wr.String()
 }
 
-func newBufLogger(sw *syncWriter) Logger {
-	return PrintfLogger(log.New(sw, "", log.LstdFlags))
-}
-
-func TestFuncPanicRecovery(t *testing.T) {
-	var buf syncWriter
-	cron := New(WithParser(secondParser),
-		WithMiddleware(Recover(newBufLogger(&buf))))
-	cron.Start()
-	defer cron.Stop()
-	cron.AddFunc("* * * * * ?", func(context.Context) error { //nolint:errcheck
-		panic("YOLO")
-	})
-
-	time.Sleep(OneSecond)
-	if !strings.Contains(buf.String(), "YOLO") {
-		t.Error("expected a panic to be logged, got none")
-	}
-}
-
-type DummyJob struct{}
-
-func (d DummyJob) Run(context.Context) error {
-	panic("YOLO")
-}
-
-func TestJobPanicRecovery(t *testing.T) {
-	var job DummyJob
-
-	var buf syncWriter
-	cron := New(WithParser(secondParser),
-		WithMiddleware(Recover(newBufLogger(&buf))))
-	cron.Start()
-	defer cron.Stop()
-	cron.AddJob("* * * * * ?", job) //nolint:errcheck
-
-	time.Sleep(OneSecond)
-	if !strings.Contains(buf.String(), "YOLO") {
-		t.Error("expected a panic to be logged, got none")
-	}
-}
-
 // Start and stop cron with no entries.
 func TestNoEntries(t *testing.T) {
 	cron := newWithSeconds()
@@ -328,6 +288,19 @@ func TestRunningMultipleSchedules(t *testing.T) {
 	}
 }
 
+func TestCron_Use(t *testing.T) {
+	cron := New()
+	assert.Len(t, cron.middlewares, 0)
+
+	cron.Use(NoopMiddleware(), NoopMiddleware(), func(next Job) Job {
+		return JobFunc(func(ctx context.Context) error {
+			return next.Run(ctx)
+		})
+	})
+
+	assert.Len(t, cron.middlewares, 3)
+}
+
 // Test that the cron is run in the local time zone (as opposed to UTC).
 func TestLocalTimezone(t *testing.T) {
 	wg := &sync.WaitGroup{}

internal/logger/buffer.go

@@ -0,0 +1,34 @@
+package logger
+
+import (
+	"bytes"
+	"log"
+	"sync"
+
+	"github.com/flc1125/go-cron/v4"
+)
+
+type Buffer struct {
+	buf bytes.Buffer
+	mu  sync.Mutex
+}
+
+func NewBuffer() *Buffer {
+	return &Buffer{}
+}
+
+func (b *Buffer) Write(p []byte) (n int, err error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.buf.Write(p)
+}
+
+func (b *Buffer) String() string {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.buf.String()
+}
+
+func NewBufferLogger(buffer *Buffer) cron.Logger {
+	return cron.PrintfLogger(log.New(buffer, "", log.LstdFlags))
+}

middleware.go

@@ -2,8 +2,6 @@ package cron
 
 import (
 	"context"
-	"fmt"
-	"runtime"
 	"sync"
 	"time"
 )
@@ -24,25 +22,11 @@ func Chain(m ...Middleware) Middleware {
 	}
 }
 
-// Recover panics in wrapped jobs and log them with the provided logger.
-// Deprecated: recovery.New()
-func Recover(logger Logger) Middleware {
+// NoopMiddleware returns a Middleware that does nothing.
+// It is useful for testing and for composing with other Middlewares.
+func NoopMiddleware() Middleware {
 	return func(j Job) Job {
-		return JobFunc(func(ctx context.Context) error {
-			defer func() {
-				if r := recover(); r != nil {
-					const size = 64 << 10
-					buf := make([]byte, size)
-					buf = buf[:runtime.Stack(buf, false)]
-					err, ok := r.(error)
-					if !ok {
-						err = fmt.Errorf("%v", r)
-					}
-					logger.Error(err, "panic", "stack", "...\n"+string(buf))
-				}
-			}()
-			return j.Run(ctx)
-		})
+		return j
 	}
 }
 

middleware/recovery/README.md

@@ -0,0 +1,28 @@
+# Recovery Middleware
+
+The `recovery` middleware is a middleware for [go-cron](https://github.com/flc1125/go-cron) that recovers from panics.
+
+## Usage
+
+```go
+package recovery_test
+
+import (
+	"context"
+
+	"github.com/flc1125/go-cron/v4"
+	"github.com/flc1125/go-cron/v4/middleware/recovery"
+)
+
+func Example() {
+	c := cron.New()
+	c.Use(recovery.New())
+
+	c.AddFunc("* * * * * ?", func(ctx context.Context) error {
+		panic("YOLO")
+	})
+
+	c.Start()
+	defer c.Stop()
+}
+```

middleware/recovery/example_test.go

@@ -0,0 +1,20 @@
+package recovery_test
+
+import (
+	"context"
+
+	"github.com/flc1125/go-cron/v4"
+	"github.com/flc1125/go-cron/v4/middleware/recovery"
+)
+
+func Example() {
+	c := cron.New()
+	c.Use(recovery.New())
+
+	_, _ = c.AddFunc("* * * * * ?", func(context.Context) error {
+		panic("YOLO")
+	})
+
+	c.Start()
+	defer c.Stop()
+}

middleware/recovery/recovery_test.go

@@ -0,0 +1,94 @@
+package recovery
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/flc1125/go-cron/v4"
+	"github.com/flc1125/go-cron/v4/internal/logger"
+)
+
+type panicJob struct{}
+
+func (p panicJob) Run(context.Context) error {
+	panic("YOLO")
+}
+
+func TestRecovery(t *testing.T) {
+	buf := logger.NewBuffer()
+	recovery := New(
+		WithLogger(logger.NewBufferLogger(buf)),
+	)
+
+	assert.NotPanics(t, func() {
+		_ = recovery(cron.JobFunc(func(context.Context) error {
+			panic("YOLO")
+		})).Run(context.Background())
+	})
+
+	assert.True(t, strings.Contains(buf.String(), "YOLO"))
+}
+
+func TestRecovery_FuncPanic(t *testing.T) {
+	buf := logger.NewBuffer()
+	c := cron.New(
+		cron.WithSeconds(),
+		cron.WithMiddleware(
+			New(
+				WithLogger(logger.NewBufferLogger(buf)),
+			),
+		),
+	)
+	c.Start()
+	defer c.Stop()
+
+	_, err := c.AddFunc("* * * * * ?", func(context.Context) error {
+		panic("YOLO")
+	})
+	assert.NoError(t, err)
+
+	time.Sleep(time.Second)
+	assert.True(t, strings.Contains(buf.String(), "YOLO"))
+}
+
+func TestRecovery_JobPanic(t *testing.T) {
+	buf := logger.NewBuffer()
+	c := cron.New(
+		cron.WithSeconds(),
+		cron.WithMiddleware(
+			New(
+				WithLogger(logger.NewBufferLogger(buf)),
+			),
+		),
+	)
+	c.Start()
+	defer c.Stop()
+
+	_, err := c.AddJob("* * * * * ?", panicJob{})
+	assert.NoError(t, err)
+
+	time.Sleep(time.Second)
+	assert.True(t, strings.Contains(buf.String(), "YOLO"))
+}
+
+func TestRecovery_ChainPanic(t *testing.T) {
+	t.Run("default panic exits job", func(*testing.T) {
+		assert.Panics(t, func() {
+			_ = cron.Chain()(panicJob{}).Run(context.Background())
+		})
+	})
+
+	t.Run("recovering job wrapper recovers", func(*testing.T) {
+		var buf logger.Buffer
+		assert.NotPanics(t, func() {
+			_ = cron.Chain(
+				New(WithLogger(logger.NewBufferLogger(&buf))),
+			)(panicJob{}).Run(context.Background())
+		})
+		assert.True(t, strings.Contains(buf.String(), "YOLO"))
+	})
+}

middleware_test.go

@@ -2,12 +2,12 @@ package cron
 
 import (
 	"context"
-	"io"
-	"log"
 	"reflect"
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func appendingJob(slice *[]int, value int) Job {
@@ -43,32 +43,6 @@ func TestChain(t *testing.T) {
 	}
 }
 
-func TestChainRecover(t *testing.T) {
-	panickingJob := JobFunc(func(context.Context) error {
-		panic("panickingJob panics")
-	})
-
-	t.Run("panic exits job by default", func(t *testing.T) {
-		defer func() {
-			if err := recover(); err == nil {
-				t.Errorf("panic expected, but none received")
-			}
-		}()
-		Chain()(panickingJob).
-			Run(context.Background()) //nolint:errcheck
-	})
-
-	t.Run("Recovering JobWrapper recovers", func(*testing.T) {
-		Chain(Recover(PrintfLogger(log.New(io.Discard, "", 0))))(panickingJob).
-			Run(context.Background()) //nolint:errcheck
-	})
-
-	t.Run("composed with the *IfStillRunning wrappers", func(*testing.T) {
-		Chain(Recover(PrintfLogger(log.New(io.Discard, "", 0))))(panickingJob).
-			Run(context.Background()) //nolint:errcheck
-	})
-}
-
 type countJob struct {
 	m       sync.Mutex
 	started int
@@ -245,3 +219,10 @@ func TestChainSkipIfStillRunning(t *testing.T) {
 		}
 	})
 }
+
+func TestMiddleware_NoopMiddleware(t *testing.T) {
+	err := NoopMiddleware()(JobFunc(func(context.Context) error {
+		return nil
+	})).Run(context.Background())
+	assert.NoError(t, err)
+}