diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 8e6a9079f..95e142237 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -420,7 +420,7 @@ func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) } if n == 0 { - return models.ErrRoutesRemoving + return models.ErrRoutesNotFound } return nil diff --git a/api/models/app.go b/api/models/app.go index 156f77b57..51942a59b 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -1,46 +1,15 @@ package models -import ( - "errors" - "fmt" -) - type Apps []*App type Tasks []*Task type FnCalls []*FnCall -var ( - ErrAppsAlreadyExists = errors.New("App already exists") - ErrAppsCreate = errors.New("Could not create app") - ErrAppsGet = errors.New("Could not get app from datastore") - ErrAppsList = errors.New("Could not list apps from datastore") - ErrAppsMissingNew = errors.New("Missing new application") - ErrAppsNameImmutable = errors.New("Could not update app - name is immutable") - ErrAppsNotFound = errors.New("App not found") - ErrAppsNothingToUpdate = errors.New("Nothing to update") - ErrAppsRemoving = errors.New("Could not remove app from datastore") - ErrAppsUpdate = errors.New("Could not update app") - ErrDeleteAppsWithRoutes = errors.New("Cannot remove apps with routes") - ErrUsableImage = errors.New("Image not found") - ErrTaskInvalidAppAndRoute = errors.New("Unable to get call for given app and route") -) - type App struct { Name string `json:"name"` Routes Routes `json:"routes,omitempty"` Config `json:"config"` } -const ( - maxAppName = 30 -) - -var ( - ErrAppsValidationMissingName = errors.New("Missing app name") - ErrAppsValidationTooLongName = fmt.Errorf("App name must be %v characters or less", maxAppName) - ErrAppsValidationInvalidName = errors.New("Invalid app name") -) - func (a *App) Validate() error { if a.Name == "" { return ErrAppsValidationMissingName diff --git a/api/models/app_wrapper.go b/api/models/app_wrapper.go index 815888261..743a9fbc6 100644 --- a/api/models/app_wrapper.go +++ b/api/models/app_wrapper.go @@ -1,27 +1,12 @@ package models -import "github.com/go-openapi/errors" - type AppWrapper struct { App *App `json:"app"` } -func (m *AppWrapper) Validate() error { - var res []error - - if err := m.validateApp(); err != nil { - // prop - res = append(res, err) - } - - if len(res) > 0 { - return errors.CompositeValidationError(res...) - } - return nil -} +func (m *AppWrapper) Validate() error { return m.validateApp() } func (m *AppWrapper) validateApp() error { - if m.App != nil { if err := m.App.Validate(); err != nil { return err diff --git a/api/models/complete.go b/api/models/complete.go index 2d3b92ac1..670047e90 100644 --- a/api/models/complete.go +++ b/api/models/complete.go @@ -2,9 +2,6 @@ package models import "github.com/go-openapi/strfmt" -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - /*Complete complete swagger:model Complete diff --git a/api/models/datastore.go b/api/models/datastore.go index a81fbc3a8..c88713be6 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -2,7 +2,6 @@ package models import ( "context" - "errors" "github.com/jmoiron/sqlx" ) @@ -47,8 +46,7 @@ type Datastore interface { // InsertRoute inserts a route. Returns ErrDatastoreEmptyRoute when route is nil, and ErrDatastoreEmptyAppName // or ErrDatastoreEmptyRoutePath for empty AppName or Path. - // Returns ErrRoutesAlreadyExists if the exact route.Path already exists, or ErrRoutesCreate if a conflicting - // route already exists. + // Returns ErrRoutesAlreadyExists if the exact route.Path already exists InsertRoute(ctx context.Context, route *Route) (*Route, error) // UpdateRoute updates route's Config and Header fields. Returns ErrDatastoreEmptyRoute when route is nil, and @@ -70,12 +68,3 @@ type Datastore interface { // GetDatabase returns the underlying sqlx database implementation GetDatabase() *sqlx.DB } - -var ( - ErrDatastoreEmptyAppName = errors.New("Missing app name") - ErrDatastoreEmptyRoutePath = errors.New("Missing route name") - ErrDatastoreEmptyApp = errors.New("Missing app") - ErrDatastoreEmptyRoute = errors.New("Missing route") - ErrDatastoreEmptyKey = errors.New("Missing key") - ErrDatastoreEmptyTaskID = errors.New("Missing task ID") -) diff --git a/api/models/error.go b/api/models/error.go index ae7dc12ad..ab4238b04 100644 --- a/api/models/error.go +++ b/api/models/error.go @@ -1,7 +1,182 @@ package models -import "errors" +import ( + "errors" + "fmt" + "net/http" +) +// TODO we can put constants all in this file too +const ( + maxAppName = 30 +) + +var ( + ErrInvalidJSON = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid JSON"), + } + ErrRunnerTimeout = err{ + code: http.StatusGatewayTimeout, + error: errors.New("Timed out"), + } + ErrRunnerRouteNotFound = err{ + code: http.StatusNotFound, + error: errors.New("Route not found on that application"), + } + ErrAppsValidationMissingName = err{ + code: http.StatusBadRequest, + error: errors.New("Missing app name"), + } + ErrAppsValidationTooLongName = err{ + code: http.StatusBadRequest, + error: fmt.Errorf("App name must be %v characters or less", maxAppName), + } + ErrAppsValidationInvalidName = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid app name"), + } + ErrAppsAlreadyExists = err{ + code: http.StatusConflict, + error: errors.New("App already exists"), + } + ErrAppsMissingNew = err{ + code: http.StatusBadRequest, + error: errors.New("Missing new application"), + } + ErrAppsNameImmutable = err{ + code: http.StatusConflict, + error: errors.New("Could not update app - name is immutable"), + } + ErrAppsNotFound = err{ + code: http.StatusNotFound, + error: errors.New("App not found"), + } + ErrDeleteAppsWithRoutes = err{ + code: http.StatusConflict, + error: errors.New("Cannot remove apps with routes"), + } + ErrDatastoreEmptyAppName = err{ + code: http.StatusBadRequest, + error: errors.New("Missing app name"), + } + ErrDatastoreEmptyRoutePath = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route name"), + } + ErrDatastoreEmptyApp = err{ + code: http.StatusBadRequest, + error: errors.New("Missing app"), + } + ErrDatastoreEmptyRoute = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route"), + } + ErrDatastoreEmptyKey = err{ + code: http.StatusBadRequest, + error: errors.New("Missing key"), + } + ErrDatastoreEmptyTaskID = err{ + code: http.StatusBadRequest, + error: errors.New("Missing task ID"), + } + ErrInvalidPayload = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid payload"), + } + ErrRoutesAlreadyExists = err{ + code: http.StatusConflict, + error: errors.New("Route already exists"), + } + ErrRoutesMissingNew = err{ + code: http.StatusBadRequest, + error: errors.New("Missing new route"), + } + ErrRoutesNotFound = err{ + code: http.StatusNotFound, + error: errors.New("Route not found"), + } + ErrRoutesPathImmutable = err{ + code: http.StatusConflict, + error: errors.New("Could not update route - path is immutable"), + } + ErrRoutesValidationFoundDynamicURL = err{ + code: http.StatusBadRequest, + error: errors.New("Dynamic URL is not allowed"), + } + ErrRoutesValidationInvalidPath = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid Path format"), + } + ErrRoutesValidationInvalidType = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid route Type"), + } + ErrRoutesValidationInvalidFormat = err{ + code: http.StatusBadRequest, + error: errors.New("Invalid route Format"), + } + ErrRoutesValidationMissingAppName = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route AppName"), + } + ErrRoutesValidationMissingImage = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route Image"), + } + ErrRoutesValidationMissingName = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route Name"), + } + ErrRoutesValidationMissingPath = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route Path"), + } + ErrRoutesValidationMissingType = err{ + code: http.StatusBadRequest, + error: errors.New("Missing route Type"), + } + ErrRoutesValidationPathMalformed = err{ + code: http.StatusBadRequest, + error: errors.New("Path malformed"), + } + ErrRoutesValidationNegativeTimeout = err{ + code: http.StatusBadRequest, + error: errors.New("Negative timeout"), + } + ErrRoutesValidationNegativeIdleTimeout = err{ + code: http.StatusBadRequest, + error: errors.New("Negative idle timeout"), + } + ErrNoSpecialHandlerFound = err{ + code: http.StatusNotFound, + error: errors.New("Path not found"), + } + ErrCallNotFound = err{ + code: http.StatusNotFound, + error: errors.New("Call not found"), + } + ErrCallLogNotFound = err{ + code: http.StatusNotFound, + error: errors.New("Call log not found"), + } +) + +// any error that implements this interface will return an API response +// with the provided status code and error message body +type APIError interface { + Code() int + error +} + +type err struct { + code int + error +} + +func (e err) Code() int { return e.code } + +// uniform error output type Error struct { Error *ErrorBody `json:"error,omitempty"` } @@ -9,7 +184,3 @@ type Error struct { func (m *Error) Validate() error { return nil } - -var ( - ErrInvalidJSON = errors.New("Invalid JSON") -) diff --git a/api/models/id_status.go b/api/models/id_status.go index 70a00bab6..c31c7a385 100644 --- a/api/models/id_status.go +++ b/api/models/id_status.go @@ -1,15 +1,11 @@ package models -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - import ( "encoding/json" strfmt "github.com/go-openapi/strfmt" "github.com/go-openapi/swag" - "github.com/go-openapi/errors" "github.com/go-openapi/validate" ) @@ -67,19 +63,7 @@ type IDStatus struct { } // Validate validates this Id status -func (m *IDStatus) Validate(formats strfmt.Registry) error { - var res []error - - if err := m.validateStatus(formats); err != nil { - // prop - res = append(res, err) - } - - if len(res) > 0 { - return errors.CompositeValidationError(res...) - } - return nil -} +func (m *IDStatus) Validate(formats strfmt.Registry) error { return m.validateStatus(formats) } var idStatusTypeStatusPropEnum []interface{} diff --git a/api/models/reason.go b/api/models/reason.go index cc647ecb4..a04f098af 100644 --- a/api/models/reason.go +++ b/api/models/reason.go @@ -1,14 +1,10 @@ package models -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - import ( "encoding/json" strfmt "github.com/go-openapi/strfmt" - "github.com/go-openapi/errors" "github.com/go-openapi/validate" ) @@ -43,15 +39,6 @@ func (m Reason) validateReasonEnum(path, location string, value Reason) error { // Validate validates this reason func (m Reason) Validate(formats strfmt.Registry) error { - var res []error - // value enum - if err := m.validateReasonEnum("", "body", m); err != nil { - return err - } - - if len(res) > 0 { - return errors.CompositeValidationError(res...) - } - return nil + return m.validateReasonEnum("", "body", m) } diff --git a/api/models/route.go b/api/models/route.go index 61b2a5428..b143a1543 100644 --- a/api/models/route.go +++ b/api/models/route.go @@ -1,13 +1,10 @@ package models import ( - "errors" "net/http" "net/url" "path" "strings" - - apiErrors "github.com/go-openapi/errors" ) const ( @@ -15,19 +12,6 @@ const ( htfnScaleDownTimeout = 30 // seconds ) -var ( - ErrInvalidPayload = errors.New("Invalid payload") - ErrRoutesAlreadyExists = errors.New("Route already exists") - ErrRoutesCreate = errors.New("Could not create route") - ErrRoutesGet = errors.New("Could not get route from datastore") - ErrRoutesList = errors.New("Could not list routes from datastore") - ErrRoutesMissingNew = errors.New("Missing new route") - ErrRoutesNotFound = errors.New("Route not found") - ErrRoutesPathImmutable = errors.New("Could not update route - path is immutable") - ErrRoutesRemoving = errors.New("Could not remove route from datastore") - ErrRoutesUpdate = errors.New("Could not update route") -) - type Routes []*Route type Route struct { @@ -43,21 +27,6 @@ type Route struct { Config `json:"config"` } -var ( - ErrRoutesValidationFoundDynamicURL = errors.New("Dynamic URL is not allowed") - ErrRoutesValidationInvalidPath = errors.New("Invalid Path format") - ErrRoutesValidationInvalidType = errors.New("Invalid route Type") - ErrRoutesValidationInvalidFormat = errors.New("Invalid route Format") - ErrRoutesValidationMissingAppName = errors.New("Missing route AppName") - ErrRoutesValidationMissingImage = errors.New("Missing route Image") - ErrRoutesValidationMissingName = errors.New("Missing route Name") - ErrRoutesValidationMissingPath = errors.New("Missing route Path") - ErrRoutesValidationMissingType = errors.New("Missing route Type") - ErrRoutesValidationPathMalformed = errors.New("Path malformed") - ErrRoutesValidationNegativeTimeout = errors.New("Negative timeout") - ErrRoutesValidationNegativeIdleTimeout = errors.New("Negative idle timeout") -) - // SetDefaults sets zeroed field to defaults. func (r *Route) SetDefaults() { if r.Memory == 0 { @@ -90,60 +59,55 @@ func (r *Route) SetDefaults() { } // Validate validates field values, skipping zeroed fields if skipZero is true. +// it returns the first error, if any. func (r *Route) Validate(skipZero bool) error { - var res []error - if !skipZero { if r.AppName == "" { - res = append(res, ErrRoutesValidationMissingAppName) - } - - if r.Image == "" { - res = append(res, ErrRoutesValidationMissingImage) + return ErrRoutesValidationMissingAppName } if r.Path == "" { - res = append(res, ErrRoutesValidationMissingPath) + return ErrRoutesValidationMissingPath + } + + if r.Image == "" { + return ErrRoutesValidationMissingImage } } if !skipZero || r.Path != "" { u, err := url.Parse(r.Path) if err != nil { - res = append(res, ErrRoutesValidationPathMalformed) + return ErrRoutesValidationPathMalformed } if strings.Contains(u.Path, ":") { - res = append(res, ErrRoutesValidationFoundDynamicURL) + return ErrRoutesValidationFoundDynamicURL } if !path.IsAbs(u.Path) { - res = append(res, ErrRoutesValidationInvalidPath) + return ErrRoutesValidationInvalidPath } } if !skipZero || r.Type != "" { if r.Type != TypeAsync && r.Type != TypeSync { - res = append(res, ErrRoutesValidationInvalidType) + return ErrRoutesValidationInvalidType } } if !skipZero || r.Format != "" { if r.Format != FormatDefault && r.Format != FormatHTTP { - res = append(res, ErrRoutesValidationInvalidFormat) + return ErrRoutesValidationInvalidFormat } } if r.Timeout < 0 { - res = append(res, ErrRoutesValidationNegativeTimeout) + return ErrRoutesValidationNegativeTimeout } if r.IdleTimeout < 0 { - res = append(res, ErrRoutesValidationNegativeIdleTimeout) - } - - if len(res) > 0 { - return apiErrors.CompositeValidationError(res...) + return ErrRoutesValidationNegativeIdleTimeout } return nil diff --git a/api/models/route_wrapper.go b/api/models/route_wrapper.go index 4420bac8a..dad82dab0 100644 --- a/api/models/route_wrapper.go +++ b/api/models/route_wrapper.go @@ -1,23 +1,10 @@ package models -import "github.com/go-openapi/errors" - type RouteWrapper struct { Route *Route `json:"route"` } -func (m *RouteWrapper) Validate(skipZero bool) error { - var res []error - - if err := m.validateRoute(skipZero); err != nil { - res = append(res, err) - } - - if len(res) > 0 { - return errors.CompositeValidationError(res...) - } - return nil -} +func (m *RouteWrapper) Validate(skipZero bool) error { return m.validateRoute(skipZero) } func (m *RouteWrapper) validateRoute(skipZero bool) error { diff --git a/api/models/runner.go b/api/models/runner.go deleted file mode 100644 index 0d51734e6..000000000 --- a/api/models/runner.go +++ /dev/null @@ -1,13 +0,0 @@ -package models - -import "errors" - -var ( - ErrRunnerRouteNotFound = errors.New("Route not found on that application") - ErrRunnerInvalidPayload = errors.New("Invalid payload") - ErrRunnerRunRoute = errors.New("Couldn't run this route in the job server") - ErrRunnerAPICantConnect = errors.New("Couldn`t connect to the job server API") - ErrRunnerAPICreateJob = errors.New("Could not create a job in job server") - ErrRunnerInvalidResponse = errors.New("Invalid response") - ErrRunnerTimeout = errors.New("Timed out") -) diff --git a/api/models/start.go b/api/models/start.go index ec5784090..0518321e8 100644 --- a/api/models/start.go +++ b/api/models/start.go @@ -2,9 +2,6 @@ package models import "github.com/go-openapi/strfmt" -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - /*Start start swagger:model Start diff --git a/api/models/task.go b/api/models/task.go index 2368e53f1..7373441c7 100644 --- a/api/models/task.go +++ b/api/models/task.go @@ -1,15 +1,9 @@ package models -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - import ( "encoding/json" - apierrors "errors" strfmt "github.com/go-openapi/strfmt" - - "github.com/go-openapi/errors" "github.com/go-openapi/validate" ) @@ -29,12 +23,6 @@ const ( FormatHTTP = "http" ) -var ( - ErrCallNotFound = apierrors.New("Call not found") - ErrCallLogNotFound = apierrors.New("Call log not found") - ErrCallLogRemoving = apierrors.New("Could not remove call log") -) - type FnCall struct { IDStatus CompletedAt strfmt.DateTime `json:"completed_at,omitempty"` @@ -167,25 +155,18 @@ type Task struct { // Validate validates this task func (m *Task) Validate(formats strfmt.Registry) error { - var res []error - - // NewTask validations will get generated automatically - if err := m.IDStatus.Validate(formats); err != nil { - res = append(res, err) + return err } if err := m.validateEnvVars(formats); err != nil { - res = append(res, err) + return err } if err := m.validateReason(formats); err != nil { - res = append(res, err) + return err } - if len(res) > 0 { - return errors.CompositeValidationError(res...) - } return nil } diff --git a/api/runner/drivers/docker/docker.go b/api/runner/drivers/docker/docker.go index 19110ff83..5adac6fe1 100644 --- a/api/runner/drivers/docker/docker.go +++ b/api/runner/drivers/docker/docker.go @@ -418,6 +418,10 @@ func (drv *DockerDriver) run(ctx context.Context, container string, task drivers err = drv.startTask(ctx, container) if err != nil { + if err == context.DeadlineExceeded { + // if there's just a timeout making the docker calls, rewrite it as such + return &runResult{start: start, status: drivers.StatusTimeout}, nil + } return nil, err } diff --git a/api/server/apps_create.go b/api/server/apps_create.go index f1d3c7474..63e51a7ac 100644 --- a/api/server/apps_create.go +++ b/api/server/apps_create.go @@ -5,38 +5,32 @@ import ( "github.com/gin-gonic/gin" "gitlab-odx.oracle.com/odx/functions/api/models" - "gitlab-odx.oracle.com/odx/functions/api/runner/common" ) func (s *Server) handleAppCreate(c *gin.Context) { ctx := c.MustGet("mctx").(MiddlewareContext) - log := common.Logger(ctx) var wapp models.AppWrapper err := c.BindJSON(&wapp) if err != nil { - log.WithError(err).Debug(models.ErrInvalidJSON) - c.JSON(http.StatusBadRequest, simpleError(models.ErrInvalidJSON)) + handleErrorResponse(c, models.ErrInvalidJSON) return } if wapp.App == nil { - log.Debug(models.ErrAppsMissingNew) - c.JSON(http.StatusBadRequest, simpleError(models.ErrAppsMissingNew)) + handleErrorResponse(c, models.ErrAppsMissingNew) return } if err := wapp.Validate(); err != nil { - log.Error(err) - c.JSON(http.StatusInternalServerError, simpleError(err)) + handleErrorResponse(c, err) return } err = s.FireBeforeAppCreate(ctx, wapp.App) if err != nil { - log.WithError(err).Error(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + handleErrorResponse(c, err) return } @@ -48,8 +42,7 @@ func (s *Server) handleAppCreate(c *gin.Context) { err = s.FireAfterAppCreate(ctx, wapp.App) if err != nil { - log.WithError(err).Error(models.ErrAppsCreate) - c.JSON(http.StatusInternalServerError, simpleError(err)) + handleErrorResponse(c, err) return } diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index 59a106493..bcfe13b35 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -17,21 +17,20 @@ func (s *Server) handleAppDelete(c *gin.Context) { routes, err := s.Datastore.GetRoutesByApp(ctx, app.Name, &models.RouteFilter{}) if err != nil { - log.WithError(err).Error(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + log.WithError(err).Error("error getting route in app delete") + handleErrorResponse(c, err) return } //TODO allow this? #528 if len(routes) > 0 { - log.WithError(err).Debug(models.ErrDeleteAppsWithRoutes) - c.JSON(http.StatusBadRequest, simpleError(models.ErrDeleteAppsWithRoutes)) + handleErrorResponse(c, models.ErrDeleteAppsWithRoutes) return } err = s.FireBeforeAppDelete(ctx, app) if err != nil { - log.WithError(err).Error(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + log.WithError(err).Error("error firing before app delete") + handleErrorResponse(c, err) return } @@ -49,8 +48,8 @@ func (s *Server) handleAppDelete(c *gin.Context) { err = s.FireAfterAppDelete(ctx, app) if err != nil { - log.WithError(err).Error(models.ErrAppsRemoving) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + log.WithError(err).Error("error firing after app delete") + handleErrorResponse(c, err) return } diff --git a/api/server/apps_test.go b/api/server/apps_test.go index 2db5053e7..588274f89 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -39,10 +39,10 @@ func TestAppCreate(t *testing.T) { {datastore.NewMock(), logs.NewMock(), "/v1/apps", ``, http.StatusBadRequest, models.ErrInvalidJSON}, {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{}`, http.StatusBadRequest, models.ErrAppsMissingNew}, {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "name": "Test" }`, http.StatusBadRequest, models.ErrAppsMissingNew}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusInternalServerError, models.ErrAppsValidationMissingName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusInternalServerError, models.ErrAppsValidationTooLongName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, - {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "" } }`, http.StatusBadRequest, models.ErrAppsValidationMissingName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "1234567890123456789012345678901" } }`, http.StatusBadRequest, models.ErrAppsValidationTooLongName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "&&%@!#$#@$" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, // success {datastore.NewMock(), logs.NewMock(), "/v1/apps", `{ "app": { "name": "teste" } }`, http.StatusOK, nil}, @@ -215,7 +215,7 @@ func TestAppUpdate(t *testing.T) { []*models.App{{ Name: "myapp", }}, nil, nil, nil, - ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusBadRequest, nil}, + ), logs.NewMock(), "/v1/apps/myapp", `{ "app": { "name": "othername" } }`, http.StatusConflict, nil}, } { rnr, cancel := testRunner(t) srv := testServer(test.mock, &mqs.Mock{}, test.logDB, rnr) diff --git a/api/server/apps_update.go b/api/server/apps_update.go index d1e564ca3..0430bdbdf 100644 --- a/api/server/apps_update.go +++ b/api/server/apps_update.go @@ -6,31 +6,26 @@ import ( "github.com/gin-gonic/gin" "gitlab-odx.oracle.com/odx/functions/api" "gitlab-odx.oracle.com/odx/functions/api/models" - "gitlab-odx.oracle.com/odx/functions/api/runner/common" ) func (s *Server) handleAppUpdate(c *gin.Context) { ctx := c.MustGet("mctx").(MiddlewareContext) - log := common.Logger(ctx) wapp := models.AppWrapper{} err := c.BindJSON(&wapp) if err != nil { - log.WithError(err).Debug(models.ErrInvalidJSON) - c.JSON(http.StatusBadRequest, simpleError(models.ErrInvalidJSON)) + handleErrorResponse(c, models.ErrInvalidJSON) return } if wapp.App == nil { - log.Debug(models.ErrAppsMissingNew) - c.JSON(http.StatusBadRequest, simpleError(models.ErrAppsMissingNew)) + handleErrorResponse(c, models.ErrAppsMissingNew) return } if wapp.App.Name != "" { - log.Debug(models.ErrAppsNameImmutable) - c.JSON(http.StatusBadRequest, simpleError(models.ErrAppsNameImmutable)) + handleErrorResponse(c, models.ErrAppsNameImmutable) return } @@ -38,8 +33,7 @@ func (s *Server) handleAppUpdate(c *gin.Context) { err = s.FireAfterAppUpdate(ctx, wapp.App) if err != nil { - log.WithError(err).Error(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + handleErrorResponse(c, err) return } @@ -51,8 +45,7 @@ func (s *Server) handleAppUpdate(c *gin.Context) { err = s.FireAfterAppUpdate(ctx, wapp.App) if err != nil { - log.WithError(err).Error(models.ErrAppsUpdate) - c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) + handleErrorResponse(c, err) return } diff --git a/api/server/call_list.go b/api/server/call_list.go index dcfef1748..0fc74d903 100644 --- a/api/server/call_list.go +++ b/api/server/call_list.go @@ -14,24 +14,24 @@ func (s *Server) handleCallList(c *gin.Context) { appName, ok := c.MustGet(api.AppName).(string) if ok && appName == "" { - c.JSON(http.StatusBadRequest, models.ErrRoutesValidationMissingAppName) + handleErrorResponse(c, models.ErrRoutesValidationMissingAppName) return } _, err := s.Datastore.GetApp(c, appName) if err != nil { - c.JSON(http.StatusNotFound, models.ErrAppsNotFound) + handleErrorResponse(c, err) return } appRoute, ok := c.MustGet(api.Path).(string) if ok && appRoute == "" { - c.JSON(http.StatusBadRequest, models.ErrRoutesValidationMissingPath) + handleErrorResponse(c, models.ErrRoutesValidationMissingPath) return } _, err = s.Datastore.GetRoute(c, appName, appRoute) if err != nil { - c.JSON(http.StatusNotFound, models.ErrRoutesNotFound) + handleErrorResponse(c, err) return } diff --git a/api/server/error_response.go b/api/server/error_response.go index 8e832c62d..5972d7eef 100644 --- a/api/server/error_response.go +++ b/api/server/error_response.go @@ -3,35 +3,31 @@ package server import ( "context" "errors" + "net/http" + "runtime/debug" + + "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "gitlab-odx.oracle.com/odx/functions/api/models" "gitlab-odx.oracle.com/odx/functions/api/runner/common" - "net/http" ) -var ErrInternalServerError = errors.New("Something unexpected happened on the server") +var ErrInternalServerError = errors.New("internal server error") 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, - models.ErrCallNotFound: http.StatusNotFound, - models.ErrCallLogNotFound: http.StatusNotFound, -} - 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)) + if aerr, ok := err.(models.APIError); ok { + log.WithFields(logrus.Fields{"code": aerr.Code()}).WithError(err).Error("api error") + c.JSON(aerr.Code(), simpleError(err)) + } else if err != nil { + // get a stack trace so we can trace this error + log.WithError(err).WithFields(logrus.Fields{"stack": string(debug.Stack())}).Error("internal server error") + c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError)) } } diff --git a/api/server/extension_points.go b/api/server/extension_points.go index ae10040c4..df5704f1d 100644 --- a/api/server/extension_points.go +++ b/api/server/extension_points.go @@ -45,11 +45,13 @@ func (s *Server) apiAppHandlerWrapperFunc(apiHandler ApiAppHandler) gin.HandlerF appName := c.Param(api.CApp) app, err := s.Datastore.GetApp(c.Request.Context(), appName) if err != nil { - c.AbortWithError(http.StatusInternalServerError, err) + handleErrorResponse(c, err) + c.Abort() return } if app == nil { - c.AbortWithStatus(http.StatusNotFound) + handleErrorResponse(c, models.ErrAppsNotFound) + c.Abort() return } diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 17a3d7545..e9d23bfb4 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -9,7 +9,6 @@ import ( "github.com/gin-gonic/gin" "gitlab-odx.oracle.com/odx/functions/api" "gitlab-odx.oracle.com/odx/functions/api/models" - "gitlab-odx.oracle.com/odx/functions/api/runner/common" ) /* handleRouteCreateOrUpdate is used to handle POST PUT and PATCH for routes. @@ -25,23 +24,20 @@ import ( */ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) { ctx := c.MustGet("mctx").(MiddlewareContext) - log := common.Logger(ctx) method := strings.ToUpper(c.Request.Method) var wroute models.RouteWrapper - err, resperr := s.bindAndValidate(ctx, c, method, &wroute) - if err != nil || resperr != nil { - log.WithError(err).Debug(resperr) - c.JSON(http.StatusBadRequest, simpleError(resperr)) + err := s.bindAndValidate(ctx, c, method, &wroute) + if err != nil { + handleErrorResponse(c, err) return } // Create the app if it does not exist. - err, resperr = s.ensureApp(ctx, &wroute, method) - if err != nil || resperr != nil { - log.WithError(err).Debug(resperr) - handleErrorResponse(c, resperr) + err = s.ensureApp(ctx, &wroute, method) + if err != nil { + handleErrorResponse(c, err) return } @@ -57,53 +53,51 @@ func (s *Server) handleRouteCreateOrUpdate(c *gin.Context) { } // ensureApp will only execute if it is on post or put. Patch is not allowed to create apps. -func (s *Server) ensureApp(ctx MiddlewareContext, wroute *models.RouteWrapper, method string) (error, error) { +func (s *Server) ensureApp(ctx MiddlewareContext, wroute *models.RouteWrapper, method string) error { if !(method == http.MethodPost || method == http.MethodPut) { - return nil, nil + return nil } - var app *models.App - var err error - app, err = s.Datastore.GetApp(ctx, wroute.Route.AppName) + app, err := s.Datastore.GetApp(ctx, wroute.Route.AppName) if err != nil && err != models.ErrAppsNotFound { - return err, models.ErrAppsGet + return err } else if app == nil { // Create a new application newapp := &models.App{Name: wroute.Route.AppName} if err = newapp.Validate(); err != nil { - return err, err + return err } err = s.FireBeforeAppCreate(ctx, newapp) if err != nil { - return err, models.ErrAppsCreate + return err } _, err = s.Datastore.InsertApp(ctx, newapp) if err != nil { - return err, models.ErrAppsCreate + return err } err = s.FireAfterAppCreate(ctx, newapp) if err != nil { - return err, models.ErrAppsCreate + return err } } - return nil, nil + return nil } /* bindAndValidate binds the RouteWrapper to the json from the request and validates that it is correct. If it is a put or patch it makes sure that the path in the url matches the provideed one in the body. Defaults are set and if patch skipZero is true for validating the RouteWrapper */ -func (s *Server) bindAndValidate(ctx context.Context, c *gin.Context, method string, wroute *models.RouteWrapper) (error, error) { +func (s *Server) bindAndValidate(ctx context.Context, c *gin.Context, method string, wroute *models.RouteWrapper) error { err := c.BindJSON(wroute) if err != nil { - return err, models.ErrInvalidJSON + return models.ErrInvalidJSON } if wroute.Route == nil { - return err, models.ErrRoutesMissingNew + return models.ErrRoutesMissingNew } wroute.Route.AppName = c.MustGet(api.AppName).(string) @@ -111,17 +105,14 @@ func (s *Server) bindAndValidate(ctx context.Context, c *gin.Context, method str p := path.Clean(c.MustGet(api.Path).(string)) if wroute.Route.Path != "" && wroute.Route.Path != p { - return models.ErrRoutesPathImmutable, models.ErrRoutesPathImmutable + return models.ErrRoutesPathImmutable } wroute.Route.Path = p } wroute.Route.SetDefaults() - if err = wroute.Validate(method == http.MethodPatch); err != nil { - return models.ErrRoutesCreate, err - } - return nil, nil + return wroute.Validate(method == http.MethodPatch) } // updateOrInsertRoute will either update or insert the route respective the method. diff --git a/api/server/routes_test.go b/api/server/routes_test.go index ce78178f1..b772f628a 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -63,7 +63,7 @@ func TestRouteCreate(t *testing.T) { {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingPath}, {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/a/routes", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidPath}, - {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), http.MethodPost, "/v1/apps/$/routes", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, {datastore.NewMockInit(nil, []*models.Route{ { @@ -84,16 +84,16 @@ func TestRoutePut(t *testing.T) { buf := setLogBuffer() for i, test := range []routeTestCase{ - // errors + // errors (NOTE: this route doesn't exist yet) {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "path": "/myroute" }`, http.StatusBadRequest, models.ErrRoutesMissingNew}, {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "path": "/myroute" } }`, http.StatusBadRequest, models.ErrRoutesValidationMissingImage}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusInternalServerError, models.ErrAppsValidationInvalidName}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType}, - {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "myroute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "diffRoute" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/$/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusBadRequest, models.ErrAppsValidationInvalidName}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "type": "invalid-type" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidType}, + {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute", "format": "invalid-format" } }`, http.StatusBadRequest, models.ErrRoutesValidationInvalidFormat}, // success {datastore.NewMock(), logs.NewMock(), http.MethodPut, "/v1/apps/a/routes/myroute", `{ "route": { "image": "funcy/hello", "path": "/myroute" } }`, http.StatusOK, nil}, @@ -259,7 +259,7 @@ func TestRouteUpdate(t *testing.T) { Path: "/myroute/do", }, }, nil, nil, - ), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusBadRequest, models.ErrRoutesPathImmutable}, + ), logs.NewMock(), http.MethodPatch, "/v1/apps/a/routes/myroute/do", `{ "route": { "path": "/otherpath" } }`, http.StatusConflict, models.ErrRoutesPathImmutable}, } { test.run(t, i, buf) } diff --git a/api/server/runner.go b/api/server/runner.go index 50f154e29..f06d635ba 100644 --- a/api/server/runner.go +++ b/api/server/runner.go @@ -3,7 +3,6 @@ package server import ( "bytes" "context" - "errors" "fmt" "io" "io/ioutil" @@ -30,7 +29,6 @@ type runnerResponse struct { func (s *Server) handleSpecial(c *gin.Context) { ctx := c.MustGet("ctx").(context.Context) - log := common.Logger(ctx) ctx = context.WithValue(ctx, api.AppName, "") c.Set(api.AppName, "") @@ -38,21 +36,15 @@ func (s *Server) handleSpecial(c *gin.Context) { c.Set(api.Path, c.Request.URL.Path) ctx, err := s.UseSpecialHandlers(ctx, c.Request, c.Writer) - if err == ErrNoSpecialHandlerFound { - log.WithError(err).Errorln("Not special handler found") - c.JSON(http.StatusNotFound, http.StatusText(http.StatusNotFound)) - return - } else if err != nil { - log.WithError(err).Errorln("Error using special handler!") - c.JSON(http.StatusInternalServerError, simpleError(errors.New("Failed to run function"))) + if err != nil { + handleErrorResponse(c, err) return } c.Set("ctx", ctx) c.Set(api.AppName, ctx.Value(api.AppName).(string)) if c.MustGet(api.AppName).(string) == "" { - log.WithError(err).Errorln("Specialhandler returned empty app name") - c.JSON(http.StatusBadRequest, simpleError(models.ErrRunnerRouteNotFound)) + handleErrorResponse(c, models.ErrRunnerRouteNotFound) return } @@ -110,23 +102,23 @@ func (s *Server) handleRequest(c *gin.Context, enqueue models.Enqueue) { path := reqRoute.Path app, err := s.Datastore.GetApp(ctx, appName) - if err != nil || app == nil { - log.WithError(err).Error(models.ErrAppsNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound)) + if err != nil { + handleErrorResponse(c, err) + return + } else if app == nil { + handleErrorResponse(c, models.ErrAppsNotFound) return } log.WithFields(logrus.Fields{"app": appName, "path": path}).Debug("Finding route on datastore") routes, err := s.loadroutes(ctx, models.RouteFilter{AppName: appName, Path: path}) if err != nil { - log.WithError(err).Error(models.ErrRoutesList) - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesList)) + handleErrorResponse(c, err) return } if len(routes) == 0 { - log.WithError(err).Error(models.ErrRunnerRouteNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrRunnerRouteNotFound)) + handleErrorResponse(c, models.ErrRunnerRouteNotFound) return } @@ -139,8 +131,7 @@ func (s *Server) handleRequest(c *gin.Context, enqueue models.Enqueue) { return } - log.Error(models.ErrRunnerRouteNotFound) - c.JSON(http.StatusNotFound, simpleError(models.ErrRunnerRouteNotFound)) + handleErrorResponse(c, models.ErrRunnerRouteNotFound) } func (s *Server) loadroutes(ctx context.Context, filter models.RouteFilter) ([]*models.Route, error) { @@ -241,8 +232,7 @@ func (s *Server) serve(ctx context.Context, c *gin.Context, appName string, rout // Read payload pl, err := ioutil.ReadAll(cfg.Stdin) if err != nil { - log.WithError(err).Error(models.ErrInvalidPayload) - c.JSON(http.StatusBadRequest, simpleError(models.ErrInvalidPayload)) + handleErrorResponse(c, models.ErrInvalidPayload) return true } // Create Task diff --git a/api/server/server.go b/api/server/server.go index a084e1466..d1a86b953 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -205,28 +205,24 @@ func (s *Server) handleTaskRequest(c *gin.Context) { case "GET": task, err := s.MQ.Reserve(ctx) if err != nil { - logrus.WithError(err).Error() - c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesList)) + handleErrorResponse(c, err) return } c.JSON(http.StatusAccepted, task) case "DELETE": body, err := ioutil.ReadAll(c.Request.Body) if err != nil { - logrus.WithError(err).Error() - c.JSON(http.StatusInternalServerError, err) + handleErrorResponse(c, err) return } var task models.Task if err = json.Unmarshal(body, &task); err != nil { - logrus.WithError(err).Error() - c.JSON(http.StatusInternalServerError, err) + handleErrorResponse(c, err) return } if err := s.MQ.Delete(ctx, &task); err != nil { - logrus.WithError(err).Error() - c.JSON(http.StatusInternalServerError, err) + handleErrorResponse(c, err) return } c.JSON(http.StatusAccepted, task) diff --git a/api/server/server_test.go b/api/server/server_test.go index 86a4884c8..51407f71d 100644 --- a/api/server/server_test.go +++ b/api/server/server_test.go @@ -123,7 +123,7 @@ func TestFullStack(t *testing.T) { {"execute myroute", "POST", "/r/myapp/myroute", `{ "name": "Teste" }`, http.StatusOK, 2}, {"execute myroute2", "POST", "/r/myapp/myroute2", `{ "name": "Teste" }`, http.StatusInternalServerError, 2}, {"delete myroute", "DELETE", "/v1/apps/myapp/routes/myroute", ``, http.StatusOK, 1}, - {"delete app (fail)", "DELETE", "/v1/apps/myapp", ``, http.StatusBadRequest, 1}, + {"delete app (fail)", "DELETE", "/v1/apps/myapp", ``, http.StatusConflict, 1}, {"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}, diff --git a/api/server/special_handler.go b/api/server/special_handler.go index bf584d088..00023a433 100644 --- a/api/server/special_handler.go +++ b/api/server/special_handler.go @@ -2,11 +2,10 @@ package server import ( "context" - "errors" "net/http" -) -var ErrNoSpecialHandlerFound = errors.New("Path not found") + "gitlab-odx.oracle.com/odx/functions/api/models" +) type SpecialHandler interface { Handle(c HandlerContext) error @@ -56,7 +55,7 @@ func (s *Server) AddSpecialHandler(handler SpecialHandler) { // UseSpecialHandlers execute all special handlers func (s *Server) UseSpecialHandlers(ctx context.Context, req *http.Request, resp http.ResponseWriter) (context.Context, error) { if len(s.specialHandlers) == 0 { - return ctx, ErrNoSpecialHandlerFound + return ctx, models.ErrNoSpecialHandlerFound } c := &SpecialHandlerContext{