Skip to content

Commit

Permalink
refactor: add errors package that wraps cockroachdb/errors (#112)
Browse files Browse the repository at this point in the history
cockroachdb/errors gives us stack traces and information for end users
with WithHint().

The reason for the 'errors' wrapper package is so as not to spread a
third party dependency all over the code and make it easier to replace
should that ever become necessary. If the API of the internal 'errors'
keeps growing and becoming closer and closer to cockroachdb/errors then
at some point the internal package might be deleted.

In schema.resolvers.go it was necessary to import the packages as
'errors2' because otherwise the automated code generation kept
overriding it.
  • Loading branch information
omarkohl committed Aug 4, 2023
1 parent 5f943f6 commit c042978
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 77 deletions.
5 changes: 3 additions & 2 deletions cleoc/cleoc/config.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package cleoc

import (
"fmt"
"strings"

"github.com/adrg/xdg"
"github.com/spf13/viper"

"github.com/cleodora-forecasting/cleodora/cleoutils/errors"
)

const DefaultConfigFileName = "cleoc"
Expand Down Expand Up @@ -37,7 +38,7 @@ func (c *Config) LoadWithViper(v *viper.Viper) error {
// Note that ConfigFileNotFoundError is not returned when
// explicitly specifying a config path (--config), which should
// (and does) cause an error if it doesn't exist.
return fmt.Errorf("error reading config file: %w", err)
return errors.Newf("error reading config file: %w", err)
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions cleoc/cleoc/forecast.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cleoc

import (
"context"
"errors"
"fmt"
"net/http"
"time"
Expand All @@ -11,20 +10,21 @@ import (
"github.com/hashicorp/go-multierror"

"github.com/cleodora-forecasting/cleodora/cleoc/gqclient"
"github.com/cleodora-forecasting/cleodora/cleoutils/errors"
)

// AddForecast creates a new forecast.
// The incoming options opts are assumed to already have been validated.
func (a *App) AddForecast(opts AddForecastOptions) error {
resolvesT, err := time.Parse(time.RFC3339, opts.Resolves)
if err != nil {
return fmt.Errorf("could not parse 'resolves': %w", err)
return errors.Newf("could not parse 'resolves': %w", err)
}
var closesT *time.Time
if opts.Closes != "" {
parsedTime, err := time.Parse(time.RFC3339, opts.Closes)
if err != nil {
return fmt.Errorf("could not parse 'closes': %w", err)
return errors.Newf("could not parse 'closes': %w", err)
}
closesT = &parsedTime
}
Expand All @@ -42,7 +42,7 @@ func (a *App) AddForecast(opts AddForecastOptions) error {

reqProbabilities, err := parseProbabilities(opts.Probabilities)
if err != nil {
return fmt.Errorf("error parsing probabilities: %w", err)
return errors.Newf("error parsing probabilities: %w", err)
}

estimate := gqclient.NewEstimate{
Expand All @@ -51,7 +51,7 @@ func (a *App) AddForecast(opts AddForecastOptions) error {
}
resp, err := gqclient.CreateForecast(ctx, client, forecast, estimate)
if err != nil {
return fmt.Errorf("error calling the API: %w", err)
return errors.Newf("error calling the API: %w", err)
}
_, err = fmt.Fprint(a.Out, resp.CreateForecast.Id+"\n")
if err != nil {
Expand Down Expand Up @@ -141,7 +141,7 @@ func (opts *AddForecastOptions) Validate() error {
if sumProbabilities != 100 {
validationErr = multierror.Append(
validationErr,
fmt.Errorf(
errors.Newf(
"all probabilities must add up to 100 (here only %v)",
sumProbabilities,
),
Expand Down
37 changes: 19 additions & 18 deletions cleosrv/cleosrv/cleosrv.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cleodora-forecasting/cleodora/cleosrv/graph"
"github.com/cleodora-forecasting/cleodora/cleosrv/graph/generated"
"github.com/cleodora-forecasting/cleodora/cleoutils"
"github.com/cleodora-forecasting/cleodora/cleoutils/errors"
)

type App struct {
Expand Down Expand Up @@ -56,7 +57,7 @@ Listening on: %s

err := os.MkdirAll(filepath.Dir(a.Config.Database), 0770)
if err != nil {
return fmt.Errorf("error making directories for database %v: %w", a.Config.Database, err)
return errors.Newf("error making directories for database %v: %w", a.Config.Database, err)
}

router := chi.NewRouter()
Expand Down Expand Up @@ -103,7 +104,7 @@ func (a *App) InitDB() (*gorm.DB, error) {

err = migrateDB(db)
if err != nil {
return nil, fmt.Errorf("migrating data: %w", err)
return nil, errors.Newf("migrating data: %w", err)
}

return db, nil
Expand All @@ -121,7 +122,7 @@ func migrateDB(db *gorm.DB) error {
return tx.Migrator().AutoMigrate(&Migrations{})
})
if err != nil {
return fmt.Errorf("auto migrating 'migrations' table: %w", err)
return errors.Newf("auto migrating 'migrations' table: %w", err)
}
// If the DB is completely new then we just insert all migrations as 'done'
// i.e. they are not really executed because the createDb() function is
Expand All @@ -130,7 +131,7 @@ func migrateDB(db *gorm.DB) error {
err := db.Transaction(func(tx *gorm.DB) error {
err := createDb(tx)
if err != nil {
return fmt.Errorf("creating tables: %w", err)
return errors.Newf("creating tables: %w", err)
}
// save all migrations as done in the DB, without executing the
// functions.
Expand All @@ -146,7 +147,7 @@ func migrateDB(db *gorm.DB) error {
}
ret := tx.Create(mEntries)
if ret.Error != nil {
return fmt.Errorf("storing migrations as done in DB: %w", ret.Error)
return errors.Newf("storing migrations as done in DB: %w", ret.Error)
}
return nil
})
Expand All @@ -157,7 +158,7 @@ func migrateDB(db *gorm.DB) error {
var count int64
ret := db.Model(&Migrations{}).Where("id = ?", m.ID).Count(&count)
if ret.Error != nil {
return fmt.Errorf("selecting migration %v: %w", m.ID, ret.Error)
return errors.Newf("selecting migration %v: %w", m.ID, ret.Error)
}
if count == 1 {
continue // migration already ran in the past
Expand All @@ -167,15 +168,15 @@ func migrateDB(db *gorm.DB) error {
if m.Up != nil {
err = m.Up(tx)
if err != nil {
return fmt.Errorf("running %v: %w", m.ID, err)
return errors.Newf("running %v: %w", m.ID, err)
}
}
ret = tx.Create(Migrations{
ID: m.ID,
Applied: time.Now().UTC(),
})
if ret.Error != nil {
return fmt.Errorf("saving migration %v: %w", m.ID, ret.Error)
return errors.Newf("saving migration %v: %w", m.ID, ret.Error)
}
fmt.Printf("Finished DB migration '%v'\n", m.ID)
return nil
Expand Down Expand Up @@ -225,7 +226,7 @@ var dbMigrations = []dbMigration{
var forecasts []Forecast
ret := db.Find(&forecasts)
if ret.Error != nil {
return fmt.Errorf("getting forecasts: %w", ret.Error)
return errors.Newf("getting forecasts: %w", ret.Error)
}
for _, f := range forecasts {
f.Created = f.Created.UTC()
Expand All @@ -252,7 +253,7 @@ var dbMigrations = []dbMigration{
}
ret = db.Save(f)
if ret.Error != nil {
return fmt.Errorf("saving %v: %w", f.ID, ret.Error)
return errors.Newf("saving %v: %w", f.ID, ret.Error)
}
}
return nil
Expand All @@ -268,14 +269,14 @@ var dbMigrations = []dbMigration{
var estimates []Estimate
ret := db.Find(&estimates)
if ret.Error != nil {
return fmt.Errorf("getting estimates: %w", ret.Error)
return errors.Newf("getting estimates: %w", ret.Error)
}
for _, e := range estimates {
e.Created = e.Created.UTC()
e.CreatedAt = e.CreatedAt.UTC()
ret = db.Save(e)
if ret.Error != nil {
return fmt.Errorf("saving %v: %w", e.ID, ret.Error)
return errors.Newf("saving %v: %w", e.ID, ret.Error)
}
}
return nil
Expand All @@ -290,13 +291,13 @@ var dbMigrations = []dbMigration{
var outcomes []Outcome
ret := db.Find(&outcomes)
if ret.Error != nil {
return fmt.Errorf("getting outcomes: %w", ret.Error)
return errors.Newf("getting outcomes: %w", ret.Error)
}
for _, o := range outcomes {
o.CreatedAt = o.CreatedAt.UTC()
ret = db.Save(o)
if ret.Error != nil {
return fmt.Errorf("saving %v: %w", o.ID, ret.Error)
return errors.Newf("saving %v: %w", o.ID, ret.Error)
}
}
return nil
Expand All @@ -311,13 +312,13 @@ var dbMigrations = []dbMigration{
var probabilities []Probability
ret := db.Find(&probabilities)
if ret.Error != nil {
return fmt.Errorf("getting probabilities: %w", ret.Error)
return errors.Newf("getting probabilities: %w", ret.Error)
}
for _, p := range probabilities {
p.CreatedAt = p.CreatedAt.UTC()
ret = db.Save(p)
if ret.Error != nil {
return fmt.Errorf("saving %v: %w", p.ID, ret.Error)
return errors.Newf("saving %v: %w", p.ID, ret.Error)
}
}
return nil
Expand Down Expand Up @@ -366,15 +367,15 @@ var dbMigrations = []dbMigration{
Scan(&estimateBriers)

if ret.Error != nil {
return fmt.Errorf("error calculating brier score: %w", ret.Error)
return errors.Newf("error calculating brier score: %w", ret.Error)
}

for _, r := range estimateBriers {
ret := db.Model(&dbmodel.Estimate{}).
Where("id = ?", r.ID).
Update("brier_score", r.Brier)
if ret.Error != nil {
return fmt.Errorf("error updating brier score: %w", ret.Error)
return errors.Newf("error updating brier score: %w", ret.Error)
}
}

Expand Down
5 changes: 3 additions & 2 deletions cleosrv/cleosrv/config.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package cleosrv

import (
"fmt"
"strings"

"github.com/adrg/xdg"
"github.com/spf13/viper"

"github.com/cleodora-forecasting/cleodora/cleoutils/errors"
)

const DefaultConfigFileName = "cleosrv"
Expand Down Expand Up @@ -41,7 +42,7 @@ func (c *Config) LoadWithViper(v *viper.Viper) error {
// Note that ConfigFileNotFoundError is not returned when
// explicitly specifying a config path (--config), which should
// (and does) cause an error if it doesn't exist.
return fmt.Errorf("error reading config file: %w", err)
return errors.Newf("error reading config file: %w", err)
}
}
}
Expand Down
28 changes: 14 additions & 14 deletions cleosrv/graph/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package graph

import (
"context"
"errors"
"fmt"
"html"
"strconv"
Expand All @@ -15,6 +14,7 @@ import (

"github.com/cleodora-forecasting/cleodora/cleosrv/dbmodel"
"github.com/cleodora-forecasting/cleodora/cleosrv/graph/model"
"github.com/cleodora-forecasting/cleodora/cleoutils/errors"
)

// This file will not be regenerated automatically.
Expand Down Expand Up @@ -55,12 +55,12 @@ func createEstimate(
estimate model.NewEstimate,
) (*model.Estimate, error) {
if err := validateNewEstimate(&estimate, false); err != nil {
return nil, fmt.Errorf("error validating NewEstimate: %w", err)
return nil, errors.Newf("error validating NewEstimate: %w", err)
}
forecast := dbmodel.Forecast{}
ret := tx.Where("id = ?", forecastID).First(&forecast)
if ret.Error != nil {
return nil, fmt.Errorf("error getting Forecast with ID %v: %w", forecastID, ret.Error)
return nil, errors.Newf("error getting Forecast with ID %v: %w", forecastID, ret.Error)
}

if estimate.Created.Before(forecast.Created) {
Expand Down Expand Up @@ -92,7 +92,7 @@ func createEstimate(
).Where("estimates.forecast_id = ?", forecastID).
Distinct().Pluck("outcomes.id", &validOutcomeIds)
if ret.Error != nil {
return nil, fmt.Errorf("error getting outcome IDs: %w", ret.Error)
return nil, errors.Newf("error getting outcome IDs: %w", ret.Error)
}

var submittedOutcomeIds []string
Expand All @@ -102,20 +102,20 @@ func createEstimate(

invalidOutcomeIds := stringSetDiff(submittedOutcomeIds, validOutcomeIds)
if len(invalidOutcomeIds) > 0 {
return nil, fmt.Errorf("invalid Outcome IDs: %v", invalidOutcomeIds)
return nil, errors.Newf("invalid Outcome IDs: %v", invalidOutcomeIds)
}

missingOutcomeIds := stringSetDiff(validOutcomeIds, submittedOutcomeIds)
if len(missingOutcomeIds) > 0 {
return nil, fmt.Errorf("missing Outcome IDs: %v", missingOutcomeIds)
return nil, errors.Newf("missing Outcome IDs: %v", missingOutcomeIds)
}

var probabilities []dbmodel.Probability

for _, p := range estimate.Probabilities {
outcomeID, err := strconv.ParseUint(*p.OutcomeID, 10, 64)
if err != nil {
return nil, fmt.Errorf("can't parse %v as uint: %w", outcomeID, err)
return nil, errors.Newf("can't parse %v as uint: %w", outcomeID, err)
}
probabilities = append(
probabilities,
Expand All @@ -134,7 +134,7 @@ func createEstimate(

err := tx.Model(&forecast).Association("Estimates").Append(&dbEstimate)
if err != nil {
return nil, fmt.Errorf("error creating Estimate: %w", err)
return nil, errors.Newf("error creating Estimate: %w", err)
}

return convertEstimateDBToGQL(dbEstimate), nil
Expand Down Expand Up @@ -192,7 +192,7 @@ func validateNewForecast(forecast *model.NewForecast) error {
if forecast.Closes.After(forecast.Resolves) {
validationErr = multierror.Append(
validationErr,
fmt.Errorf(
errors.Newf(
"'Closes' can't be set to a later date than 'Resolves'. "+
"Closes is '%v'. Resolves is '%v'",
*forecast.Closes,
Expand All @@ -217,7 +217,7 @@ func validateNewForecast(forecast *model.NewForecast) error {
if forecast.Resolves.Before(*forecast.Created) {
validationErr = multierror.Append(
validationErr,
fmt.Errorf(
errors.Newf(
"'Resolves' can't be set to an earlier date than 'Created'. "+
"Resolves is '%v'. Created is '%v'",
forecast.Resolves,
Expand Down Expand Up @@ -275,7 +275,7 @@ func validateNewEstimate(estimate *model.NewEstimate, duringCreateForecast bool)
if _, ok := existingOutcomes[p.Outcome.Text]; ok {
validationErr = multierror.Append(
validationErr,
fmt.Errorf("outcome '%v' is a duplicate", p.Outcome.Text),
errors.Newf("outcome '%v' is a duplicate", p.Outcome.Text),
)
}
existingOutcomes[p.Outcome.Text] = true
Expand All @@ -302,15 +302,15 @@ func validateNewEstimate(estimate *model.NewEstimate, duringCreateForecast bool)
if p.Value < 0 || p.Value > 100 {
validationErr = multierror.Append(
validationErr,
fmt.Errorf("probabilities must be between 0 and 100, not %v", p.Value),
errors.Newf("probabilities must be between 0 and 100, not %v", p.Value),
)
}
sumProbabilities += p.Value
}
if sumProbabilities != 100 {
validationErr = multierror.Append(
validationErr,
fmt.Errorf("probabilities must add up to 100, not %v", sumProbabilities),
errors.Newf("probabilities must add up to 100, not %v", sumProbabilities),
)
}

Expand All @@ -320,7 +320,7 @@ func validateNewEstimate(estimate *model.NewEstimate, duringCreateForecast bool)
if estimate.Created.After(now) {
validationErr = multierror.Append(
validationErr,
fmt.Errorf(
errors.Newf(
"'created' can't be in the future: %v",
estimate.Created,
),
Expand Down
Loading

0 comments on commit c042978

Please sign in to comment.