From e30ca29bcd13683f57f6d52e309f4e8ded8b79ed Mon Sep 17 00:00:00 2001 From: Brandur Leach Date: Mon, 20 Nov 2023 06:19:03 -0800 Subject: [PATCH] Document `AttemptError` and rename `AttemptError.Num` to `Attempt` (#43) Two error-related changes: * Document `AttemptError` and all its fields. This one's been on my list for a while and just brings it up to par with other exported structs like `JobRow`. * Rename `AttemptError.Num` to `Attempt`. I hadn't gone in attending to change this one, but as I was looking into what `Num` was, I realized that it'd more obvious if it matched up with the field that it comes from on `JobRow`. By having the same name as `JobRow.Attempt`, it's very obvious that these two things are related. --- CHANGELOG.md | 2 ++ client_test.go | 8 ++++---- internal/dbadapter/db_adapter_test.go | 4 ++-- internal/dbsqlc/error.go | 8 ++++---- internal/dbsqlc/river_job_ext.go | 8 ++++---- internal/maintenance/rescuer.go | 8 ++++---- job_executor.go | 8 ++++---- job_executor_test.go | 4 ++-- rivertype/job_row.go | 22 ++++++++++++++++++---- 9 files changed, 44 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36920466..5e9c72b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Constants renamed so that adjectives like `Default` and `Min` become suffixes instead of prefixes. So for example, `DefaultFetchCooldown` becomes `FetchCooldownDefault`. +- Rename `AttemptError.Num` to `AttemptError.Attempt` to better fit with the name of `JobRow.Attempt`. +- Document `JobState`, `AttemptError`, and all fields its fields. ## [0.0.6] - 2023-11-19 diff --git a/client_test.go b/client_test.go index deb2cf01..01be1d81 100644 --- a/client_test.go +++ b/client_test.go @@ -1181,10 +1181,10 @@ func Test_Client_Maintenance(t *testing.T) { for i := 0; i < errorCount; i++ { var err error errorsBytes[i], err = json.Marshal(rivertype.AttemptError{ - At: time.Now(), - Error: "mocked error", - Num: i + 1, - Trace: "none", + At: time.Now(), + Attempt: i + 1, + Error: "mocked error", + Trace: "none", }) require.NoError(t, err) } diff --git a/internal/dbadapter/db_adapter_test.go b/internal/dbadapter/db_adapter_test.go index d0cf4733..2691e32b 100644 --- a/internal/dbadapter/db_adapter_test.go +++ b/internal/dbadapter/db_adapter_test.go @@ -611,7 +611,7 @@ func Test_StandardAdapter_JobSetErroredIfRunning(t *testing.T) { tNow := time.Now() errPayload, err := json.Marshal(dbsqlc.AttemptError{ - Num: 1, At: tNow.UTC(), Error: "fake error", Trace: "foo.go:123\nbar.go:456", + Attempt: 1, At: tNow.UTC(), Error: "fake error", Trace: "foo.go:123\nbar.go:456", }) require.NoError(t, err) @@ -655,7 +655,7 @@ func Test_StandardAdapter_JobSetErroredIfRunning(t *testing.T) { // validate error payload: require.Len(t, jAfter.Errors, 1) require.Equal(t, bundle.baselineTime.UTC(), jAfter.Errors[0].At) - require.Equal(t, uint16(1), jAfter.Errors[0].Num) + require.Equal(t, uint16(1), jAfter.Errors[0].Attempt) require.Equal(t, "fake error", jAfter.Errors[0].Error) require.Equal(t, "foo.go:123\nbar.go:456", jAfter.Errors[0].Trace) }) diff --git a/internal/dbsqlc/error.go b/internal/dbsqlc/error.go index 3403e751..5a9ad974 100644 --- a/internal/dbsqlc/error.go +++ b/internal/dbsqlc/error.go @@ -3,8 +3,8 @@ package dbsqlc import "time" type AttemptError struct { - At time.Time `json:"at"` - Error string `json:"error"` - Num uint16 `json:"num"` - Trace string `json:"trace"` + At time.Time `json:"at"` + Attempt uint16 `json:"attempt"` + Error string `json:"error"` + Trace string `json:"trace"` } diff --git a/internal/dbsqlc/river_job_ext.go b/internal/dbsqlc/river_job_ext.go index d3811ab6..f059841c 100644 --- a/internal/dbsqlc/river_job_ext.go +++ b/internal/dbsqlc/river_job_ext.go @@ -33,9 +33,9 @@ func JobRowFromInternal(internal *RiverJob) *rivertype.JobRow { func AttemptErrorFromInternal(e *AttemptError) rivertype.AttemptError { return rivertype.AttemptError{ - At: e.At, - Error: e.Error, - Num: int(e.Num), - Trace: e.Trace, + At: e.At, + Attempt: int(e.Attempt), + Error: e.Error, + Trace: e.Trace, } } diff --git a/internal/maintenance/rescuer.go b/internal/maintenance/rescuer.go index 6e7e67c7..cf3faaab 100644 --- a/internal/maintenance/rescuer.go +++ b/internal/maintenance/rescuer.go @@ -173,10 +173,10 @@ func (s *Rescuer) runOnce(ctx context.Context) (*rescuerRunOnceResult, error) { rescueManyParams.ID[i] = job.ID rescueManyParams.Error[i], err = json.Marshal(rivertype.AttemptError{ - At: now, - Error: "Stuck job rescued by Rescuer", - Num: max(int(job.Attempt), 0), - Trace: "TODO", + At: now, + Attempt: max(int(job.Attempt), 0), + Error: "Stuck job rescued by Rescuer", + Trace: "TODO", }) if err != nil { return nil, fmt.Errorf("error marshaling error JSON: %w", err) diff --git a/job_executor.go b/job_executor.go index 7736ed22..7419b430 100644 --- a/job_executor.go +++ b/job_executor.go @@ -270,10 +270,10 @@ func (e *jobExecutor) reportError(ctx context.Context) { } attemptErr := rivertype.AttemptError{ - At: e.start, - Error: errorStr, - Num: e.JobRow.Attempt, - Trace: string(e.result.PanicTrace), + At: e.start, + Attempt: e.JobRow.Attempt, + Error: errorStr, + Trace: string(e.result.PanicTrace), } errData, err := json.Marshal(attemptErr) diff --git a/job_executor_test.go b/job_executor_test.go index 1b6f214d..5f2ccaab 100644 --- a/job_executor_test.go +++ b/job_executor_test.go @@ -244,7 +244,7 @@ func TestJobExecutor_Execute(t *testing.T) { require.Equal(t, dbsqlc.JobStateRetryable, job.State) require.Len(t, job.Errors, 1) require.Equal(t, baselineTime, job.Errors[0].At) - require.Equal(t, uint16(1), job.Errors[0].Num) + require.Equal(t, uint16(1), job.Errors[0].Attempt) require.Equal(t, "job error", job.Errors[0].Error) require.Equal(t, job.Errors[0].Trace, "") }) @@ -307,8 +307,8 @@ func TestJobExecutor_Execute(t *testing.T) { require.Equal(t, dbsqlc.JobStateCancelled, job.State) require.Len(t, job.Errors, 1) require.WithinDuration(t, time.Now(), job.Errors[0].At, 2*time.Second) + require.Equal(t, uint16(1), job.Errors[0].Attempt) require.Equal(t, "throw away this job", job.Errors[0].Error) - require.Equal(t, uint16(1), job.Errors[0].Num) require.Equal(t, "", job.Errors[0].Trace) }) diff --git a/rivertype/job_row.go b/rivertype/job_row.go index a4d9bfb1..b7b0bafe 100644 --- a/rivertype/job_row.go +++ b/rivertype/job_row.go @@ -96,6 +96,8 @@ type JobRow struct { // metadata []byte } +// JobState is the state of a job. Jobs start as `available` or `scheduled`, and +// if all goes well eventually transition to `completed` as they're worked. type JobState string const ( @@ -108,9 +110,21 @@ const ( JobStateScheduled JobState = "scheduled" ) +// AttemptError is an error from a single job attempt that failed due to an +// error or a panic. type AttemptError struct { - At time.Time `json:"at"` - Error string `json:"error"` - Num int `json:"num"` - Trace string `json:"trace"` + // At is the time at which the error occurred. + At time.Time `json:"at"` + + // Attempt is the attempt number on which the error occurred (maps to + // Attempt on a job row). + Attempt int `json:"attempt"` + + // Error contains the stringified error of an error returned from a job or a + // panic value in case of a panic. + Error string `json:"error"` + + // Trace contains a stack trace from a job that panicked. The trace is + // produced by invoking `debug.Trace()`. + Trace string `json:"trace"` }