From c0aed2fbb07ddfdb9ffdeb887bbf94e1e8241eb3 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Thu, 13 Jul 2017 21:14:01 -0700 Subject: [PATCH] mask errors in api response, log real error we had this _almost_ right, in that we were trying, but we weren't masking the error from the user response for any error we don't intend to show. this also adds a stack trace from any internal server errors, so that we might be able to track them down in the future (looking at you, 'context deadline exceeded'). in addition, this adds a new `models.APIError` interface which all of the errors in `models` now implement, and can be caught easily / added to easily. the front end now does no status rewriting based on api errors, now when we get a non-nil error we can call `handleResponse(c, err)` with it and if it's a proper error, return it to the user with the right status code, otherwise log a stack trace and return `internal server error`. this cleans up a lot of the front end code. also rewrites start task ctx deadline exceeded as timeout. with iw we had async tasks so we could start the clock later and it didn't matter, but now with sync tasks time out sometimes just making docker calls, and we want the task status to show up as timed out. we may want to just catch all this above in addition to this, but this seems like the right thing to do. remove squishing together errors. this was weird, now we return the first error for the purposes of using the new err interface. removed a lot of 5xx errors that really should have been 4xx errors. changed some of the 400 errors to 409 errors, since they are from sending in conflicting info and not a malformed request. removed unused errors / useless errors (many were used for logging, and didn't provide any context. now with stack traces we don't need context as much in the logs). --- api/datastore/sql/sql.go | 2 +- api/models/app.go | 31 ----- api/models/app_wrapper.go | 17 +-- api/models/complete.go | 3 - api/models/datastore.go | 13 +- api/models/error.go | 181 +++++++++++++++++++++++++++- api/models/id_status.go | 18 +-- api/models/reason.go | 15 +-- api/models/route.go | 64 +++------- api/models/route_wrapper.go | 15 +-- api/models/runner.go | 13 -- api/models/start.go | 3 - api/models/task.go | 25 +--- api/runner/drivers/docker/docker.go | 4 + api/server/apps_create.go | 17 +-- api/server/apps_delete.go | 15 ++- api/server/apps_test.go | 10 +- api/server/apps_update.go | 17 +-- api/server/call_list.go | 8 +- api/server/error_response.go | 28 ++--- api/server/extension_points.go | 6 +- api/server/routes_create_update.go | 49 +++----- api/server/routes_test.go | 16 +-- api/server/runner.go | 34 ++---- api/server/server.go | 12 +- api/server/server_test.go | 2 +- api/server/special_handler.go | 7 +- 27 files changed, 293 insertions(+), 332 deletions(-) delete mode 100644 api/models/runner.go diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index b7fb6f274..b00208e4e 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{