From 9d49dbc4658e6aed04a982ccffbd2ebdea55ec78 Mon Sep 17 00:00:00 2001 From: Victor Vrantchan Date: Tue, 20 Dec 2016 13:35:22 -0500 Subject: [PATCH] change the implementation of ErrNotFound and AlreadyExists to a struct type (#665) with an exposed interface. Not checking for a specific sentinel error reduces coupling between packages and allows adding context like the resource ID and resource type. --- server/datastore/inmem/app.go | 7 +-- server/datastore/inmem/campaigns.go | 7 ++- server/datastore/inmem/errors.go | 59 +++++++++++++++++++++ server/datastore/inmem/hosts.go | 11 ++-- server/datastore/inmem/invites.go | 13 ++--- server/datastore/inmem/labels.go | 3 +- server/datastore/inmem/packs.go | 9 ++-- server/datastore/inmem/password_reset.go | 16 +++--- server/datastore/inmem/queries.go | 8 +-- server/datastore/inmem/scheduled_queries.go | 7 ++- server/datastore/inmem/sessions.go | 13 ++--- server/datastore/inmem/users.go | 14 ++--- server/datastore/mysql/errors.go | 59 +++++++++++++++++++++ server/errors/errors.go | 13 +---- server/kolide/datastore.go | 13 +++++ server/service/service_invites.go | 4 +- server/service/service_invites_test.go | 10 ++-- server/service/service_sessions.go | 8 ++- server/service/service_users_test.go | 15 ++++-- server/service/transport_error.go | 40 +++++++++----- 20 files changed, 232 insertions(+), 97 deletions(-) create mode 100644 server/datastore/inmem/errors.go create mode 100644 server/datastore/mysql/errors.go diff --git a/server/datastore/inmem/app.go b/server/datastore/inmem/app.go index 7e3ba82299..452f425c36 100644 --- a/server/datastore/inmem/app.go +++ b/server/datastore/inmem/app.go @@ -1,9 +1,6 @@ package inmem -import ( - "github.com/kolide/kolide-ose/server/errors" - "github.com/kolide/kolide-ose/server/kolide" -) +import "github.com/kolide/kolide-ose/server/kolide" func (d *Datastore) NewAppConfig(info *kolide.AppConfig) (*kolide.AppConfig, error) { d.mtx.Lock() @@ -22,7 +19,7 @@ func (d *Datastore) AppConfig() (*kolide.AppConfig, error) { return d.orginfo, nil } - return nil, errors.ErrNotFound + return nil, notFound("AppConfig") } func (d *Datastore) SaveAppConfig(info *kolide.AppConfig) error { diff --git a/server/datastore/inmem/campaigns.go b/server/datastore/inmem/campaigns.go index b105dc5f33..fa63e87be5 100644 --- a/server/datastore/inmem/campaigns.go +++ b/server/datastore/inmem/campaigns.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -24,7 +23,7 @@ func (d *Datastore) DistributedQueryCampaign(id uint) (*kolide.DistributedQueryC campaign, ok := d.distributedQueryCampaigns[id] if !ok { - return nil, errors.ErrNotFound + return nil, notFound("DistributedQueryCampaign").WithID(id) } return &campaign, nil @@ -35,7 +34,7 @@ func (d *Datastore) SaveDistributedQueryCampaign(camp *kolide.DistributedQueryCa defer d.mtx.Unlock() if _, ok := d.distributedQueryCampaigns[camp.ID]; !ok { - return errors.ErrNotFound + return notFound("DistributedQueryCampaign").WithID(camp.ID) } d.distributedQueryCampaigns[camp.ID] = *camp @@ -80,7 +79,7 @@ func (d *Datastore) NewDistributedQueryExecution(exec *kolide.DistributedQueryEx for _, e := range d.distributedQueryExecutions { if exec.HostID == e.HostID && exec.DistributedQueryCampaignID == e.DistributedQueryCampaignID { fmt.Printf("%+v -- %+v\n", exec, d.distributedQueryExecutions) - return exec, errors.ErrExists + return exec, alreadyExists("DistributedQueryExecution", exec.HostID) } } diff --git a/server/datastore/inmem/errors.go b/server/datastore/inmem/errors.go new file mode 100644 index 0000000000..63b0adb009 --- /dev/null +++ b/server/datastore/inmem/errors.go @@ -0,0 +1,59 @@ +package inmem + +import "fmt" + +type notFoundError struct { + ID uint + Message string + ResourceType string +} + +func notFound(kind string) *notFoundError { + return ¬FoundError{ + ResourceType: kind, + } +} + +func (e *notFoundError) Error() string { + if e.ID == 0 { + return fmt.Sprintf("%s was not found in the datastore", e.ResourceType) + } + if e.Message != "" { + return fmt.Sprintf("%s, %s was not found in the datastore", e.ResourceType, e.Message) + } + return fmt.Sprintf("%s %d was not found in the datastore", e.ResourceType, e.ID) +} + +func (e *notFoundError) WithID(id uint) error { + e.ID = id + return e +} + +func (e *notFoundError) WithMessage(msg string) error { + e.Message = msg + return e +} + +func (e *notFoundError) IsNotFound() bool { + return true +} + +type existsError struct { + ID uint + ResourceType string +} + +func alreadyExists(kind string, id uint) error { + return &existsError{ + ID: id, + ResourceType: kind, + } +} + +func (e *existsError) Error() string { + return fmt.Sprintf("%s %d already exists in the datastore", e.ResourceType, e.ID) +} + +func (e *existsError) IsExists() bool { + return true +} diff --git a/server/datastore/inmem/hosts.go b/server/datastore/inmem/hosts.go index 2c5d85ae55..95a15e3cec 100644 --- a/server/datastore/inmem/hosts.go +++ b/server/datastore/inmem/hosts.go @@ -6,7 +6,6 @@ import ( "strings" "time" - kolide_errors "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -16,7 +15,7 @@ func (d *Datastore) NewHost(host *kolide.Host) (*kolide.Host, error) { for _, h := range d.hosts { if host.NodeKey == h.NodeKey || host.UUID == h.UUID { - return nil, kolide_errors.ErrExists + return nil, alreadyExists("Host", host.ID) } } @@ -31,7 +30,7 @@ func (d *Datastore) SaveHost(host *kolide.Host) error { defer d.mtx.Unlock() if _, ok := d.hosts[host.ID]; !ok { - return kolide_errors.ErrNotFound + return notFound("Host").WithID(host.ID) } for _, nic := range host.NetworkInterfaces { @@ -50,7 +49,7 @@ func (d *Datastore) DeleteHost(host *kolide.Host) error { defer d.mtx.Unlock() if _, ok := d.hosts[host.ID]; !ok { - return kolide_errors.ErrNotFound + return notFound("Host").WithID(host.ID) } delete(d.hosts, host.ID) @@ -63,7 +62,7 @@ func (d *Datastore) Host(id uint) (*kolide.Host, error) { host, ok := d.hosts[id] if !ok { - return nil, kolide_errors.ErrNotFound + return nil, notFound("Host").WithID(id) } return host, nil @@ -161,7 +160,7 @@ func (d *Datastore) AuthenticateHost(nodeKey string) (*kolide.Host, error) { } } - return nil, kolide_errors.ErrNotFound + return nil, notFound("AuthenticateHost") } func (d *Datastore) MarkHostSeen(host *kolide.Host, t time.Time) error { diff --git a/server/datastore/inmem/invites.go b/server/datastore/inmem/invites.go index dfa4e1b8aa..145acc054d 100644 --- a/server/datastore/inmem/invites.go +++ b/server/datastore/inmem/invites.go @@ -1,10 +1,10 @@ package inmem import ( + "fmt" "sort" "time" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -15,7 +15,7 @@ func (d *Datastore) NewInvite(invite *kolide.Invite) (*kolide.Invite, error) { for _, in := range d.invites { if in.Email == invite.Email { - return nil, errors.ErrExists + return nil, alreadyExists("Invite", invite.ID) } } @@ -76,7 +76,7 @@ func (d *Datastore) Invite(id uint) (*kolide.Invite, error) { if invite, ok := d.invites[id]; ok { return invite, nil } - return nil, errors.ErrNotFound + return nil, alreadyExists("Invite", id) } // InviteByEmail retrieves an invite for a specific email address. @@ -89,7 +89,8 @@ func (d *Datastore) InviteByEmail(email string) (*kolide.Invite, error) { return invite, nil } } - return nil, errors.ErrNotFound + return nil, notFound("Invite"). + WithMessage(fmt.Sprintf("with email address: %s", email)) } // SaveInvite saves an invitation in the datastore. @@ -98,7 +99,7 @@ func (d *Datastore) SaveInvite(invite *kolide.Invite) error { defer d.mtx.Unlock() if _, ok := d.invites[invite.ID]; !ok { - return errors.ErrNotFound + return notFound("Invite").WithID(invite.ID) } d.invites[invite.ID] = invite @@ -111,7 +112,7 @@ func (d *Datastore) DeleteInvite(invite *kolide.Invite) error { defer d.mtx.Unlock() if _, ok := d.invites[invite.ID]; !ok { - return errors.ErrNotFound + return notFound("Invite").WithID(invite.ID) } delete(d.invites, invite.ID) return nil diff --git a/server/datastore/inmem/labels.go b/server/datastore/inmem/labels.go index 479b616ae8..6f9c3b985c 100644 --- a/server/datastore/inmem/labels.go +++ b/server/datastore/inmem/labels.go @@ -7,7 +7,6 @@ import ( "strings" "time" - kolide_errors "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" "github.com/patrickmn/sortutil" ) @@ -18,7 +17,7 @@ func (d *Datastore) NewLabel(label *kolide.Label) (*kolide.Label, error) { d.mtx.Lock() for _, l := range d.labels { if l.Name == label.Name { - return nil, kolide_errors.ErrExists + return nil, alreadyExists("Label", l.ID) } } diff --git a/server/datastore/inmem/packs.go b/server/datastore/inmem/packs.go index d46e9bede0..998601e71d 100644 --- a/server/datastore/inmem/packs.go +++ b/server/datastore/inmem/packs.go @@ -3,7 +3,6 @@ package inmem import ( "sort" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -12,7 +11,7 @@ func (d *Datastore) NewPack(pack *kolide.Pack) (*kolide.Pack, error) { for _, q := range d.packs { if pack.Name == q.Name { - return nil, errors.ErrExists + return nil, alreadyExists("Pack", q.ID) } } @@ -28,7 +27,7 @@ func (d *Datastore) NewPack(pack *kolide.Pack) (*kolide.Pack, error) { func (d *Datastore) SavePack(pack *kolide.Pack) error { if _, ok := d.packs[pack.ID]; !ok { - return errors.ErrNotFound + return notFound("Pack").WithID(pack.ID) } d.mtx.Lock() @@ -40,7 +39,7 @@ func (d *Datastore) SavePack(pack *kolide.Pack) error { func (d *Datastore) DeletePack(pid uint) error { if _, ok := d.packs[pid]; !ok { - return errors.ErrNotFound + return notFound("Pack").WithID(pid) } d.mtx.Lock() @@ -55,7 +54,7 @@ func (d *Datastore) Pack(id uint) (*kolide.Pack, error) { pack, ok := d.packs[id] d.mtx.Unlock() if !ok { - return nil, errors.ErrNotFound + return nil, notFound("Pack").WithID(id) } return pack, nil diff --git a/server/datastore/inmem/password_reset.go b/server/datastore/inmem/password_reset.go index f7608d4e69..e8c4b73cba 100644 --- a/server/datastore/inmem/password_reset.go +++ b/server/datastore/inmem/password_reset.go @@ -1,7 +1,8 @@ package inmem import ( - "github.com/kolide/kolide-ose/server/errors" + "fmt" + "github.com/kolide/kolide-ose/server/kolide" ) @@ -19,7 +20,7 @@ func (d *Datastore) SavePasswordResetRequest(req *kolide.PasswordResetRequest) e defer d.mtx.Unlock() if _, ok := d.passwordResets[req.ID]; !ok { - return errors.ErrNotFound + return notFound("PasswordResetRequest").WithID(req.ID) } d.passwordResets[req.ID] = req @@ -31,7 +32,7 @@ func (d *Datastore) DeletePasswordResetRequest(req *kolide.PasswordResetRequest) defer d.mtx.Unlock() if _, ok := d.passwordResets[req.ID]; !ok { - return errors.ErrNotFound + return notFound("PasswordResetRequest").WithID(req.ID) } delete(d.passwordResets, req.ID) @@ -58,7 +59,7 @@ func (d *Datastore) FindPassswordResetByID(id uint) (*kolide.PasswordResetReques return req, nil } - return nil, errors.ErrNotFound + return nil, notFound("PasswordResetRequest").WithID(id) } func (d *Datastore) FindPassswordResetsByUserID(userID uint) ([]*kolide.PasswordResetRequest, error) { @@ -73,7 +74,8 @@ func (d *Datastore) FindPassswordResetsByUserID(userID uint) ([]*kolide.Password } if len(resets) == 0 { - return nil, errors.ErrNotFound + return nil, notFound("PasswordResetRequest"). + WithMessage(fmt.Sprintf("for user id %d", userID)) } return resets, nil @@ -89,7 +91,7 @@ func (d *Datastore) FindPassswordResetByToken(token string) (*kolide.PasswordRes } } - return nil, errors.ErrNotFound + return nil, notFound("PasswordResetRequest") } func (d *Datastore) FindPassswordResetByTokenAndUserID(token string, userID uint) (*kolide.PasswordResetRequest, error) { @@ -102,5 +104,5 @@ func (d *Datastore) FindPassswordResetByTokenAndUserID(token string, userID uint } } - return nil, errors.ErrNotFound + return nil, notFound("PasswordResetRequest") } diff --git a/server/datastore/inmem/queries.go b/server/datastore/inmem/queries.go index 6df6bb8c37..62dafd4191 100644 --- a/server/datastore/inmem/queries.go +++ b/server/datastore/inmem/queries.go @@ -15,7 +15,7 @@ func (d *Datastore) NewQuery(query *kolide.Query) (*kolide.Query, error) { for _, q := range d.queries { if query.Name == q.Name { - return nil, errors.ErrExists + return nil, alreadyExists("Query", q.ID) } } @@ -30,7 +30,7 @@ func (d *Datastore) SaveQuery(query *kolide.Query) error { defer d.mtx.Unlock() if _, ok := d.queries[query.ID]; !ok { - return errors.ErrNotFound + return notFound("Query").WithID(query.ID) } d.queries[query.ID] = query @@ -42,7 +42,7 @@ func (d *Datastore) DeleteQuery(query *kolide.Query) error { defer d.mtx.Unlock() if _, ok := d.queries[query.ID]; !ok { - return errors.ErrNotFound + return notFound("Query").WithID(query.ID) } delete(d.queries, query.ID) @@ -79,7 +79,7 @@ func (d *Datastore) Query(id uint) (*kolide.Query, error) { query, ok := d.queries[id] if !ok { - return nil, errors.ErrNotFound + return nil, notFound("Query").WithID(id) } query.AuthorName = d.getUserNameByID(query.AuthorID) diff --git a/server/datastore/inmem/scheduled_queries.go b/server/datastore/inmem/scheduled_queries.go index 06c94492be..e644217249 100644 --- a/server/datastore/inmem/scheduled_queries.go +++ b/server/datastore/inmem/scheduled_queries.go @@ -3,7 +3,6 @@ package inmem import ( "sort" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -24,7 +23,7 @@ func (d *Datastore) SaveScheduledQuery(sq *kolide.ScheduledQuery) (*kolide.Sched defer d.mtx.Unlock() if _, ok := d.scheduledQueries[sq.ID]; !ok { - return nil, errors.ErrNotFound + return nil, notFound("ScheduledQuery").WithID(sq.ID) } d.scheduledQueries[sq.ID] = sq @@ -36,7 +35,7 @@ func (d *Datastore) DeleteScheduledQuery(id uint) error { defer d.mtx.Unlock() if _, ok := d.scheduledQueries[id]; !ok { - return errors.ErrNotFound + return notFound("ScheduledQuery").WithID(id) } delete(d.scheduledQueries, id) @@ -49,7 +48,7 @@ func (d *Datastore) ScheduledQuery(id uint) (*kolide.ScheduledQuery, error) { sq, ok := d.scheduledQueries[id] if !ok { - return nil, errors.ErrNotFound + return nil, notFound("ScheduledQuery").WithID(id) } sq.Name = d.queries[sq.QueryID].Name diff --git a/server/datastore/inmem/sessions.go b/server/datastore/inmem/sessions.go index 8c24b58d95..2bfe401969 100644 --- a/server/datastore/inmem/sessions.go +++ b/server/datastore/inmem/sessions.go @@ -1,9 +1,9 @@ package inmem import ( + "fmt" "time" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -16,7 +16,7 @@ func (d *Datastore) SessionByKey(key string) (*kolide.Session, error) { return session, nil } } - return nil, errors.ErrNotFound + return nil, notFound("Session") } func (d *Datastore) SessionByID(id uint) (*kolide.Session, error) { @@ -26,7 +26,7 @@ func (d *Datastore) SessionByID(id uint) (*kolide.Session, error) { if session, ok := d.sessions[id]; ok { return session, nil } - return nil, errors.ErrNotFound + return nil, notFound("Session").WithID(id) } func (d *Datastore) ListSessionsForUser(id uint) ([]*kolide.Session, error) { @@ -40,7 +40,8 @@ func (d *Datastore) ListSessionsForUser(id uint) ([]*kolide.Session, error) { } } if len(sessions) == 0 { - return nil, errors.ErrNotFound + return nil, notFound("Session"). + WithMessage(fmt.Sprintf("for user id %d", id)) } return sessions, nil } @@ -61,7 +62,7 @@ func (d *Datastore) NewSession(session *kolide.Session) (*kolide.Session, error) func (d *Datastore) DestroySession(session *kolide.Session) error { if _, ok := d.sessions[session.ID]; !ok { - return errors.ErrNotFound + return notFound("Session").WithID(session.ID) } delete(d.sessions, session.ID) return nil @@ -79,7 +80,7 @@ func (d *Datastore) DestroyAllSessionsForUser(id uint) error { func (d *Datastore) MarkSessionAccessed(session *kolide.Session) error { session.AccessedAt = time.Now().UTC() if _, ok := d.sessions[session.ID]; !ok { - return errors.ErrNotFound + return notFound("Session").WithID(session.ID) } d.sessions[session.ID] = session return nil diff --git a/server/datastore/inmem/users.go b/server/datastore/inmem/users.go index 80435e0aad..20f628fc4e 100644 --- a/server/datastore/inmem/users.go +++ b/server/datastore/inmem/users.go @@ -1,9 +1,9 @@ package inmem import ( + "fmt" "sort" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" ) @@ -13,7 +13,7 @@ func (d *Datastore) NewUser(user *kolide.User) (*kolide.User, error) { for _, in := range d.users { if in.Username == user.Username { - return nil, errors.ErrExists + return nil, alreadyExists("User", in.ID) } } @@ -33,7 +33,8 @@ func (d *Datastore) User(username string) (*kolide.User, error) { } } - return nil, errors.ErrNotFound + return nil, notFound("User"). + WithMessage(fmt.Sprintf("with username %s", username)) } func (d *Datastore) ListUsers(opt kolide.ListOptions) ([]*kolide.User, error) { @@ -87,7 +88,8 @@ func (d *Datastore) UserByEmail(email string) (*kolide.User, error) { } } - return nil, errors.ErrNotFound + return nil, notFound("User"). + WithMessage(fmt.Sprintf("with email address %s", email)) } func (d *Datastore) UserByID(id uint) (*kolide.User, error) { @@ -98,7 +100,7 @@ func (d *Datastore) UserByID(id uint) (*kolide.User, error) { return user, nil } - return nil, errors.ErrNotFound + return nil, notFound("User").WithID(id) } func (d *Datastore) SaveUser(user *kolide.User) error { @@ -106,7 +108,7 @@ func (d *Datastore) SaveUser(user *kolide.User) error { defer d.mtx.Unlock() if _, ok := d.users[user.ID]; !ok { - return errors.ErrNotFound + return notFound("User").WithID(user.ID) } d.users[user.ID] = user diff --git a/server/datastore/mysql/errors.go b/server/datastore/mysql/errors.go new file mode 100644 index 0000000000..9e156f1b8f --- /dev/null +++ b/server/datastore/mysql/errors.go @@ -0,0 +1,59 @@ +package mysql + +import "fmt" + +type notFoundError struct { + ID uint + Message string + ResourceType string +} + +func notFound(kind string) *notFoundError { + return ¬FoundError{ + ResourceType: kind, + } +} + +func (e *notFoundError) Error() string { + if e.ID == 0 { + return fmt.Sprintf("%s was not found in the datastore", e.ResourceType) + } + if e.Message != "" { + return fmt.Sprintf("%s, %s was not found in the datastore", e.ResourceType, e.Message) + } + return fmt.Sprintf("%s %d was not found in the datastore", e.ResourceType, e.ID) +} + +func (e *notFoundError) WithID(id uint) error { + e.ID = id + return e +} + +func (e *notFoundError) WithMessage(msg string) error { + e.Message = msg + return e +} + +func (e *notFoundError) IsNotFound() bool { + return true +} + +type existsError struct { + ID uint + ResourceType string +} + +func alreadyExists(kind string, id uint) error { + return &existsError{ + ID: id, + ResourceType: kind, + } +} + +func (e *existsError) Error() string { + return fmt.Sprintf("%s %d already exists in the datastore", e.ResourceType, e.ID) +} + +func (e *existsError) IsExists() bool { + return true +} diff --git a/server/errors/errors.go b/server/errors/errors.go index c0e47b28e6..6b1674495d 100644 --- a/server/errors/errors.go +++ b/server/errors/errors.go @@ -1,17 +1,6 @@ package errors -import ( - goerrs "errors" - "net/http" -) - -var ( - // ErrNotFound is returned when the datastore resource cannot be found - ErrNotFound = goerrs.New("resource not found") - - // ErrExists is returned when creating a datastore resource that already exists - ErrExists = goerrs.New("resource already created") -) +import "net/http" // Kolide's internal representation for errors. It can be used to wrap another // error (stored in Err), and additionally contains fields for public diff --git a/server/kolide/datastore.go b/server/kolide/datastore.go index 2bc472b665..d19d8959e1 100644 --- a/server/kolide/datastore.go +++ b/server/kolide/datastore.go @@ -17,3 +17,16 @@ type Datastore interface { Drop() error Migrate() error } + +// NotFoundError is returned when the datastore resource cannot be found. +type NotFoundError interface { + error + IsNotFound() bool +} + +// AlreadyExists is returned when creating a datastore resource that already +// exists. +type AlreadyExistsError interface { + error + IsExists() bool +} diff --git a/server/service/service_invites.go b/server/service/service_invites.go index ef5d2ea292..794b56bb90 100644 --- a/server/service/service_invites.go +++ b/server/service/service_invites.go @@ -3,7 +3,6 @@ package service import ( "encoding/base64" - kolide_errors "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" "golang.org/x/net/context" ) @@ -14,7 +13,8 @@ func (svc service) InviteNewUser(ctx context.Context, payload kolide.InvitePaylo if err == nil { return nil, newInvalidArgumentError("email", "a user with this account already exists") } - if err != kolide_errors.ErrNotFound { + + if _, ok := err.(kolide.NotFoundError); !ok { return nil, err } diff --git a/server/service/service_invites_test.go b/server/service/service_invites_test.go index 77d4fa92a0..eebe92688e 100644 --- a/server/service/service_invites_test.go +++ b/server/service/service_invites_test.go @@ -1,6 +1,7 @@ package service import ( + "errors" "testing" "golang.org/x/net/context" @@ -8,7 +9,6 @@ import ( "github.com/WatchBeam/clock" "github.com/kolide/kolide-ose/server/config" "github.com/kolide/kolide-ose/server/datastore/inmem" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" "github.com/stretchr/testify/assert" ) @@ -44,7 +44,7 @@ func TestInviteNewUser(t *testing.T) { InvitedBy: &nosuchAdminID, Admin: boolPtr(false), }, - wantErr: errors.ErrNotFound, + wantErr: errors.New("User 999 was not found in the datastore"), }, { payload: kolide.InvitePayload{ @@ -67,9 +67,11 @@ func TestInviteNewUser(t *testing.T) { for _, tt := range inviteTests { t.Run("", func(t *testing.T) { invite, err := svc.InviteNewUser(context.Background(), tt.payload) - assert.Equal(t, tt.wantErr, err) - if err != nil { + if tt.wantErr != nil { + assert.Equal(t, tt.wantErr.Error(), err.Error()) return + } else { + assert.Nil(t, err) } assert.Equal(t, *tt.payload.InvitedBy, invite.InvitedBy) }) diff --git a/server/service/service_sessions.go b/server/service/service_sessions.go index c20e697ea2..eafbbdc1c2 100644 --- a/server/service/service_sessions.go +++ b/server/service/service_sessions.go @@ -7,18 +7,16 @@ import ( "time" "github.com/kolide/kolide-ose/server/contexts/viewer" - "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" "golang.org/x/net/context" ) func (svc service) Login(ctx context.Context, username, password string) (*kolide.User, string, error) { user, err := svc.userByEmailOrUsername(username) - switch err { - case nil: - case errors.ErrNotFound: + if _, ok := err.(kolide.NotFoundError); ok { return nil, "", authError{reason: "no such user"} - default: + } + if err != nil { return nil, "", err } if !user.Enabled { diff --git a/server/service/service_users_test.go b/server/service/service_users_test.go index c339a027f6..493d3895c5 100644 --- a/server/service/service_users_test.go +++ b/server/service/service_users_test.go @@ -8,7 +8,6 @@ import ( "github.com/kolide/kolide-ose/server/config" "github.com/kolide/kolide-ose/server/contexts/viewer" "github.com/kolide/kolide-ose/server/datastore/inmem" - kolide_errors "github.com/kolide/kolide-ose/server/errors" "github.com/kolide/kolide-ose/server/kolide" "github.com/WatchBeam/clock" @@ -157,7 +156,7 @@ func TestCreateUser(t *testing.T) { NeedsPasswordReset: boolPtr(true), Admin: boolPtr(false), InviteToken: &invites["admin2@example.com"].Token, - wantErr: kolide_errors.ErrNotFound, + wantErr: errors.New("Invite was not found in the datastore"), }, { Username: stringPtr("admin3"), @@ -190,7 +189,9 @@ func TestCreateUser(t *testing.T) { AdminForcedPasswordReset: tt.NeedsPasswordReset, } user, err := svc.NewUser(ctx, payload) - require.Equal(t, tt.wantErr, err) + if tt.wantErr != nil { + require.Equal(t, tt.wantErr.Error(), err.Error()) + } if err != nil { // skip rest of the test if error is not nil return @@ -321,7 +322,7 @@ func TestResetPassword(t *testing.T) { { // bad token token: "dcbaz", newPassword: "123cat!", - wantErr: kolide_errors.ErrNotFound, + wantErr: errors.New("PasswordResetRequest was not found in the datastore"), }, { // missing token newPassword: "123cat!", @@ -353,7 +354,11 @@ func TestResetPassword(t *testing.T) { assert.Nil(t, err) serr := svc.ResetPassword(ctx, tt.token, tt.newPassword) - assert.Equal(t, tt.wantErr, pkg_errors.Cause(serr)) + if tt.wantErr != nil { + assert.Equal(t, tt.wantErr.Error(), pkg_errors.Cause(serr).Error()) + } else { + assert.Nil(t, serr) + } }) } } diff --git a/server/service/transport_error.go b/server/service/transport_error.go index 542c33a3a5..5e38b0c4d2 100644 --- a/server/service/transport_error.go +++ b/server/service/transport_error.go @@ -5,7 +5,6 @@ import ( "net/http" kithttp "github.com/go-kit/kit/transport/http" - "github.com/kolide/kolide-ose/server/errors" "golang.org/x/net/context" ) @@ -94,10 +93,34 @@ func encodeError(ctx context.Context, err error, w http.ResponseWriter) { return } + type notFoundError interface { + error + IsNotFound() bool + } + if e, ok := err.(notFoundError); ok { + je := jsonError{ + Message: "Resource Not Found", + Error: e.Error(), + } + w.WriteHeader(http.StatusNotFound) + enc.Encode(je) + } + + type existsError interface { + error + IsExists() bool + } + if e, ok := err.(existsError); ok { + je := jsonError{ + Message: "Resource Already Exists", + Error: e.Error(), + } + w.WriteHeader(http.StatusConflict) + enc.Encode(je) + } + // Other errors switch domain { - case "service": - w.WriteHeader(codeFromErr(err)) case kithttp.DomainDecode: w.WriteHeader(http.StatusBadRequest) case kithttp.DomainDo: @@ -109,14 +132,3 @@ func encodeError(ctx context.Context, err error, w http.ResponseWriter) { "error": err.Error(), }) } - -func codeFromErr(err error) int { - switch err { - case errors.ErrNotFound: - return http.StatusNotFound - case errors.ErrExists: - return http.StatusConflict - default: - return http.StatusInternalServerError - } -}