Skip to content

Commit

Permalink
Document AttemptError and rename AttemptError.Num to Attempt (#43)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brandur authored Nov 20, 2023
1 parent 19c9315 commit e30ca29
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 28 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/dbadapter/db_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})
Expand Down
8 changes: 4 additions & 4 deletions internal/dbsqlc/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
8 changes: 4 additions & 4 deletions internal/dbsqlc/river_job_ext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
8 changes: 4 additions & 4 deletions internal/maintenance/rescuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions job_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions job_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
})
Expand Down Expand Up @@ -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)
})

Expand Down
22 changes: 18 additions & 4 deletions rivertype/job_row.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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"`
}

0 comments on commit e30ca29

Please sign in to comment.