diff --git a/CHANGELOG.md b/CHANGELOG.md index cf96ccb2..e9224429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Config.TestOnly` has been added. It disables various features in the River client like staggered maintenance service start that are useful in production, but may be somewhat harmful in tests because they make start/stop slower. [PR #414](https://github.com/riverqueue/river/pull/414). +### Changed + +⚠️ Version 0.9.0 has a small breaking change in `ErrorHandler`. As before, we try never to make breaking changes, but this one was deemed quite important because `ErrorHandler` was fundamentally lacking important functionality. + +- **Breaking change:** Add stack trace to `ErrorHandler.HandlePanicFunc`. Fixing code only requires adding a new `trace string` argument to `HandlePanicFunc`. [PR #423](https://github.com/riverqueue/river/pull/423). + + ``` go + # before + HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult + + # after + HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult + ``` + ### Fixed - Pausing or resuming a queue that was already paused or not paused respectively no longer returns `rivertype.ErrNotFound`. The same goes for pausing or resuming using the all queues string (`*`) when no queues are in the database (previously that also returned `rivertype.ErrNotFound`). [PR #408](https://github.com/riverqueue/river/pull/408). diff --git a/client_test.go b/client_test.go index e89a3890..5f2852c2 100644 --- a/client_test.go +++ b/client_test.go @@ -2300,7 +2300,7 @@ func Test_Client_ErrorHandler(t *testing.T) { var panicHandlerCalled bool config.ErrorHandler = &testErrorHandler{ - HandlePanicFunc: func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { + HandlePanicFunc: func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { require.Equal(t, "panic val", panicVal) panicHandlerCalled = true return &ErrorHandlerResult{} diff --git a/error_handler.go b/error_handler.go index 9b6b7050..a80f0233 100644 --- a/error_handler.go +++ b/error_handler.go @@ -20,7 +20,7 @@ type ErrorHandler interface { // // Context is descended from the one used to start the River client that // worked the job. - HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult + HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult } type ErrorHandlerResult struct { diff --git a/example_error_handler_test.go b/example_error_handler_test.go index 90f8dcf3..d268c150 100644 --- a/example_error_handler_test.go +++ b/example_error_handler_test.go @@ -22,7 +22,7 @@ func (*CustomErrorHandler) HandleError(ctx context.Context, job *rivertype.JobRo return nil } -func (*CustomErrorHandler) HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any) *river.ErrorHandlerResult { +func (*CustomErrorHandler) HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *river.ErrorHandlerResult { fmt.Printf("Job panicked with: %v\n", panicVal) // Either function can also set the job to be immediately cancelled. diff --git a/job_executor.go b/job_executor.go index 177caab9..400f6e46 100644 --- a/job_executor.go +++ b/job_executor.go @@ -100,7 +100,7 @@ var ErrJobCancelledRemotely = JobCancel(errors.New("job cancelled remotely")) type jobExecutorResult struct { Err error NextRetry time.Time - PanicTrace []byte + PanicTrace string PanicVal any } @@ -175,7 +175,7 @@ func (e *jobExecutor) execute(ctx context.Context) (res *jobExecutorResult) { ) res = &jobExecutorResult{ - PanicTrace: debug.Stack(), + PanicTrace: string(debug.Stack()), PanicVal: recovery, } } @@ -234,7 +234,7 @@ func (e *jobExecutor) invokeErrorHandler(ctx context.Context, res *jobExecutorRe case res.PanicVal != nil: errorHandlerRes = invokeAndHandlePanic("HandlePanic", func() *ErrorHandlerResult { - return e.ErrorHandler.HandlePanic(ctx, e.JobRow, res.PanicVal) + return e.ErrorHandler.HandlePanic(ctx, e.JobRow, res.PanicVal, res.PanicTrace) }) } @@ -316,7 +316,7 @@ func (e *jobExecutor) reportError(ctx context.Context, res *jobExecutorResult) { At: e.start, Attempt: e.JobRow.Attempt, Error: res.ErrorStr(), - Trace: string(res.PanicTrace), + Trace: res.PanicTrace, } errData, err := json.Marshal(attemptErr) diff --git a/job_executor_test.go b/job_executor_test.go index e1ed00b3..101e7e9e 100644 --- a/job_executor_test.go +++ b/job_executor_test.go @@ -89,14 +89,16 @@ type testErrorHandler struct { HandleErrorFunc func(ctx context.Context, job *rivertype.JobRow, err error) *ErrorHandlerResult HandlePanicCalled bool - HandlePanicFunc func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult + HandlePanicFunc func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult } // Test handler with no-ops for both error handling functions. func newTestErrorHandler() *testErrorHandler { return &testErrorHandler{ HandleErrorFunc: func(ctx context.Context, job *rivertype.JobRow, err error) *ErrorHandlerResult { return nil }, - HandlePanicFunc: func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { return nil }, + HandlePanicFunc: func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { + return nil + }, } } @@ -105,9 +107,9 @@ func (h *testErrorHandler) HandleError(ctx context.Context, job *rivertype.JobRo return h.HandleErrorFunc(ctx, job, err) } -func (h *testErrorHandler) HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { +func (h *testErrorHandler) HandlePanic(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { h.HandlePanicCalled = true - return h.HandlePanicFunc(ctx, job, panicVal) + return h.HandlePanicFunc(ctx, job, panicVal, trace) } func TestJobExecutor_Execute(t *testing.T) { @@ -565,8 +567,10 @@ func TestJobExecutor_Execute(t *testing.T) { executor, bundle := setup(t) executor.WorkUnit = newWorkUnitFactoryWithCustomRetry(func() error { panic("panic val") }, nil).MakeUnit(bundle.jobRow) - bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { + bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { require.Equal(t, "panic val", panicVal) + require.Contains(t, trace, "runtime/debug.Stack()\n") + return nil } @@ -586,7 +590,7 @@ func TestJobExecutor_Execute(t *testing.T) { executor, bundle := setup(t) executor.WorkUnit = newWorkUnitFactoryWithCustomRetry(func() error { panic("panic val") }, nil).MakeUnit(bundle.jobRow) - bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { + bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { return &ErrorHandlerResult{SetCancelled: true} } @@ -606,7 +610,7 @@ func TestJobExecutor_Execute(t *testing.T) { executor, bundle := setup(t) executor.WorkUnit = newWorkUnitFactoryWithCustomRetry(func() error { panic("panic val") }, nil).MakeUnit(bundle.jobRow) - bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any) *ErrorHandlerResult { + bundle.errorHandler.HandlePanicFunc = func(ctx context.Context, job *rivertype.JobRow, panicVal any, trace string) *ErrorHandlerResult { panic("panic handler panicked!") }