diff --git a/api/datastore/bolt/bolt.go b/api/datastore/bolt/bolt.go index b1600e716..a7428b69c 100644 --- a/api/datastore/bolt/bolt.go +++ b/api/datastore/bolt/bolt.go @@ -229,6 +229,8 @@ func (ds *BoltDatastore) GetApp(ctx context.Context, name string) (*models.App, return err } res = app + } else { + return models.ErrAppsNotFound } return nil }) @@ -239,15 +241,11 @@ func (ds *BoltDatastore) GetApp(ctx context.Context, name string) (*models.App, } func (ds *BoltDatastore) getRouteBucketForApp(tx *bolt.Tx, appName string) (*bolt.Bucket, error) { - var err error // todo: should this be reversed? Make a bucket for each app that contains sub buckets for routes, etc bp := tx.Bucket(ds.routesBucket) b := bp.Bucket([]byte(appName)) if b == nil { - b, err = bp.CreateBucket([]byte(appName)) - if err != nil { - return nil, err - } + return nil, models.ErrAppsNotFound } return b, nil } @@ -418,6 +416,10 @@ func (ds *BoltDatastore) GetRoute(ctx context.Context, appName, routePath string } v := b.Get([]byte(routePath)) + if v == nil { + return models.ErrRoutesNotFound + } + if v != nil { err = json.Unmarshal(v, &route) } diff --git a/api/datastore/bolt_test.go b/api/datastore/bolt_test.go index a33fa98b4..464c30b5b 100644 --- a/api/datastore/bolt_test.go +++ b/api/datastore/bolt_test.go @@ -67,7 +67,7 @@ func TestBolt(t *testing.T) { app, err := ds.GetApp(ctx, testApp.Name) if err != nil { t.Log(buf.String()) - t.Fatalf("Test GetApp: error: %s", err) + t.Fatalf("Test GetApp: unexpected error: %s", err) } if app.Name != testApp.Name { t.Log(buf.String()) @@ -101,9 +101,9 @@ func TestBolt(t *testing.T) { t.Fatalf("Test RemoveApp: error: %s", err) } app, err = ds.GetApp(ctx, testApp.Name) - if err != nil { + if err != models.ErrAppsNotFound { t.Log(buf.String()) - t.Fatalf("Test GetApp: error: %s", err) + t.Fatalf("Test GetApp: expected error to be `%v`, but it was `%v`", models.ErrAppsNotFound, err) } if app != nil { t.Log(buf.String()) @@ -240,11 +240,7 @@ func TestBolt(t *testing.T) { } route, err = ds.GetRoute(ctx, testRoute.AppName, testRoute.Path) - if err != nil { - t.Log(buf.String()) - t.Fatalf("Test GetRoute: error: %s", err) - } - if route != nil { + if err != models.ErrRoutesNotFound { t.Log(buf.String()) t.Fatalf("Test RemoveApp: failed to remove the route") } diff --git a/api/server/apps_create.go b/api/server/apps_create.go index 6b40d128c..d3e6ff6c8 100644 --- a/api/server/apps_create.go +++ b/api/server/apps_create.go @@ -42,13 +42,8 @@ func (s *Server) handleAppCreate(c *gin.Context) { } app, err := s.Datastore.InsertApp(ctx, wapp.App) - if err == models.ErrAppsAlreadyExists { - log.WithError(err).Debug(models.ErrAppsCreate) - c.JSON(http.StatusConflict, simpleError(err)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index 1a745f8f5..3f592a863 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -37,13 +37,8 @@ func (s *Server) handleAppDelete(c *gin.Context) { } err = s.Datastore.RemoveApp(ctx, app.Name) - if err == models.ErrAppsNotFound { - log.WithError(err).Debug(models.ErrAppsRemoving) - c.JSON(http.StatusNotFound, simpleError(err)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/apps_get.go b/api/server/apps_get.go index a724bb4b7..bebef326f 100644 --- a/api/server/apps_get.go +++ b/api/server/apps_get.go @@ -6,24 +6,15 @@ import ( "github.com/gin-gonic/gin" "github.com/iron-io/functions/api" - "github.com/iron-io/functions/api/models" - "github.com/iron-io/runner/common" ) func (s *Server) handleAppGet(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) appName := c.MustGet(api.AppName).(string) app, err := s.Datastore.GetApp(ctx, appName) - - if err != nil && err != models.ErrAppsNotFound { - log.WithError(err).Error(models.ErrAppsGet) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsGet)) - return - } else if app == nil { - log.WithError(err).Error(models.ErrAppsNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/apps_list.go b/api/server/apps_list.go index 46a7d839a..dbd89164c 100644 --- a/api/server/apps_list.go +++ b/api/server/apps_list.go @@ -6,19 +6,16 @@ import ( "github.com/gin-gonic/gin" "github.com/iron-io/functions/api/models" - "github.com/iron-io/runner/common" ) func (s *Server) handleAppList(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) filter := &models.AppFilter{} apps, err := s.Datastore.GetApps(ctx, filter) if err != nil { - log.WithError(err).Error(models.ErrAppsList) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + handleErrorResponse(c, err) return } diff --git a/api/server/apps_update.go b/api/server/apps_update.go index aaeb6ebc8..42231c6a9 100644 --- a/api/server/apps_update.go +++ b/api/server/apps_update.go @@ -45,13 +45,8 @@ func (s *Server) handleAppUpdate(c *gin.Context) { } app, err := s.Datastore.UpdateApp(ctx, wapp.App) - if err == models.ErrAppsNotFound { - log.WithError(err).Debug(models.ErrAppsUpdate) - c.JSON(http.StatusNotFound, simpleError(err)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/error_response.go b/api/server/error_response.go new file mode 100644 index 000000000..02ac15533 --- /dev/null +++ b/api/server/error_response.go @@ -0,0 +1,35 @@ +package server + +import ( + "context" + "errors" + "github.com/gin-gonic/gin" + "github.com/iron-io/functions/api/models" + "github.com/iron-io/runner/common" + "net/http" +) + +var ErrInternalServerError = errors.New("Something unexpected happened on the server") + +func simpleError(err error) *models.Error { + return &models.Error{Error: &models.ErrorBody{Message: err.Error()}} +} + +var errStatusCode = map[error]int{ + models.ErrAppsNotFound: http.StatusNotFound, + models.ErrAppsAlreadyExists: http.StatusConflict, + models.ErrRoutesNotFound: http.StatusNotFound, + models.ErrRoutesAlreadyExists: http.StatusConflict, +} + +func handleErrorResponse(c *gin.Context, err error) { + ctx := c.MustGet("ctx").(context.Context) + log := common.Logger(ctx) + log.Error(err) + + if code, ok := errStatusCode[err]; ok { + c.JSON(code, simpleError(err)) + } else { + c.JSON(http.StatusInternalServerError, simpleError(err)) + } +} diff --git a/api/server/routes_create.go b/api/server/routes_create.go index 7e29420af..0408a7e74 100644 --- a/api/server/routes_create.go +++ b/api/server/routes_create.go @@ -90,13 +90,8 @@ func (s *Server) handleRouteCreate(c *gin.Context) { } route, err := s.Datastore.InsertRoute(ctx, wroute.Route) - if err == models.ErrRoutesAlreadyExists { - log.WithError(err).Debug(models.ErrRoutesCreate) - c.JSON(http.StatusConflict, simpleError(models.ErrRoutesAlreadyExists)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrRoutesCreate) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/routes_delete.go b/api/server/routes_delete.go index 52e428648..6e4619b87 100644 --- a/api/server/routes_delete.go +++ b/api/server/routes_delete.go @@ -7,25 +7,16 @@ import ( "github.com/gin-gonic/gin" "github.com/iron-io/functions/api" - "github.com/iron-io/functions/api/models" - "github.com/iron-io/runner/common" ) func (s *Server) handleRouteDelete(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) appName := c.MustGet(api.AppName).(string) routePath := path.Clean(c.MustGet(api.Path).(string)) if err := s.Datastore.RemoveRoute(ctx, appName, routePath); err != nil { - if err == models.ErrRoutesNotFound { - log.WithError(err).Debug(models.ErrRoutesRemoving) - c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound)) - } else { - log.WithError(err).Error(models.ErrRoutesRemoving) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving)) - } + handleErrorResponse(c, err) return } diff --git a/api/server/routes_get.go b/api/server/routes_get.go index 011df6e51..af063d3d4 100644 --- a/api/server/routes_get.go +++ b/api/server/routes_get.go @@ -7,25 +7,17 @@ import ( "github.com/gin-gonic/gin" "github.com/iron-io/functions/api" - "github.com/iron-io/functions/api/models" - "github.com/iron-io/runner/common" ) func (s *Server) handleRouteGet(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) appName := c.MustGet(api.AppName).(string) routePath := path.Clean(c.MustGet(api.Path).(string)) route, err := s.Datastore.GetRoute(ctx, appName, routePath) - if err != nil && err != models.ErrRoutesNotFound { - log.WithError(err).Error(models.ErrRoutesGet) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) - return - } else if route == nil { - log.Debug(models.ErrRoutesNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/routes_list.go b/api/server/routes_list.go index 49dcdfce3..18c3b2d78 100644 --- a/api/server/routes_list.go +++ b/api/server/routes_list.go @@ -7,12 +7,10 @@ import ( "github.com/gin-gonic/gin" "github.com/iron-io/functions/api" "github.com/iron-io/functions/api/models" - "github.com/iron-io/runner/common" ) func (s *Server) handleRouteList(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) filter := &models.RouteFilter{} @@ -28,13 +26,8 @@ func (s *Server) handleRouteList(c *gin.Context) { routes, err = s.Datastore.GetRoutes(ctx, filter) } - if err == models.ErrAppsNotFound { - log.WithError(err).Debug(models.ErrRoutesGet) - c.JSON(http.StatusNotFound, simpleError(err)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrRoutesGet) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/routes_update.go b/api/server/routes_update.go index 326d8d3bf..dbd470b87 100644 --- a/api/server/routes_update.go +++ b/api/server/routes_update.go @@ -52,13 +52,8 @@ func (s *Server) handleRouteUpdate(c *gin.Context) { } route, err := s.Datastore.UpdateRoute(ctx, wroute.Route) - if err == models.ErrRoutesNotFound { - log.WithError(err).Debug(models.ErrRoutesUpdate) - c.JSON(http.StatusNotFound, simpleError(err)) - return - } else if err != nil { - log.WithError(err).Error(models.ErrRoutesUpdate) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesUpdate)) + if err != nil { + handleErrorResponse(c, err) return } diff --git a/api/server/server.go b/api/server/server.go index deed75b3a..8b264d737 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -3,7 +3,6 @@ package server import ( "context" "encoding/json" - "errors" "fmt" "io/ioutil" "net" @@ -253,12 +252,6 @@ func (s *Server) bindHandlers() { engine.NoRoute(s.handleSpecial) } -var ErrInternalServerError = errors.New("Something unexpected happened on the server") - -func simpleError(err error) *models.Error { - return &models.Error{&models.ErrorBody{Message: err.Error()}} -} - type appResponse struct { Message string `json:"message"` App *models.App `json:"app"` diff --git a/api/server/server_test.go b/api/server/server_test.go index aa1d1dc13..a848121be 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -132,7 +132,7 @@ func TestFullStack(t *testing.T) { {"delete myroute2", "DELETE", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0}, {"delete app (success)", "DELETE", "/v1/apps/myapp", ``, http.StatusOK, 0}, {"get deleted app", "GET", "/v1/apps/myapp", ``, http.StatusNotFound, 0}, - {"get delete route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusInternalServerError, 0}, + {"get deleteds route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusNotFound, 0}, } { _, rec := routerRequest(t, srv.Router, test.method, test.path, bytes.NewBuffer([]byte(test.body)))