From cd3593182ab6e631756f8550604549b01a179bb7 Mon Sep 17 00:00:00 2001 From: John Guo Date: Sat, 7 Aug 2021 10:44:57 +0800 Subject: [PATCH] improve package gerror/ghttp for error code handling --- errors/gerror/gerror.go | 24 ++++++++++++++++++------ errors/gerror/gerror_code.go | 2 ++ errors/gerror/gerror_error.go | 2 +- net/ghttp/ghttp_func.go | 6 +++--- net/ghttp/ghttp_request_param.go | 9 +++++---- net/ghttp/ghttp_request_param_page.go | 19 +++++++++++++------ net/ghttp/ghttp_server.go | 2 +- net/ghttp/ghttp_server_handler.go | 6 +++--- net/ghttp/ghttp_server_router_serve.go | 7 ++++--- net/ghttp/ghttp_unit_config_test.go | 2 +- 10 files changed, 51 insertions(+), 28 deletions(-) diff --git a/errors/gerror/gerror.go b/errors/gerror/gerror.go index 29e14bb14..7e1e9af03 100644 --- a/errors/gerror/gerror.go +++ b/errors/gerror/gerror.go @@ -165,10 +165,14 @@ func NewCodef(code int, format string, args ...interface{}) error { // NewCodeSkip creates and returns an error which has error code and is formatted from given text. // The parameter specifies the stack callers skipped amount. -func NewCodeSkip(code, skip int, text string) error { +func NewCodeSkip(code, skip int, text ...string) error { + errText := "" + if len(text) > 0 { + errText = text[0] + } return &Error{ stack: callers(skip), - text: text, + text: errText, code: code, } } @@ -185,14 +189,18 @@ func NewCodeSkipf(code, skip int, format string, args ...interface{}) error { // WrapCode wraps error with code and text. // It returns nil if given err is nil. -func WrapCode(code int, err error, text string) error { +func WrapCode(code int, err error, text ...string) error { if err == nil { return nil } + errText := "" + if len(text) > 0 { + errText = text[0] + } return &Error{ error: err, stack: callers(), - text: text, + text: errText, code: code, } } @@ -214,14 +222,18 @@ func WrapCodef(code int, err error, format string, args ...interface{}) error { // WrapCodeSkip wraps error with code and text. // It returns nil if given err is nil. // The parameter specifies the stack callers skipped amount. -func WrapCodeSkip(code, skip int, err error, text string) error { +func WrapCodeSkip(code, skip int, err error, text ...string) error { if err == nil { return nil } + errText := "" + if len(text) > 0 { + errText = text[0] + } return &Error{ error: err, stack: callers(skip), - text: text, + text: errText, code: code, } } diff --git a/errors/gerror/gerror_code.go b/errors/gerror/gerror_code.go index c0eecc0f4..699deaa11 100644 --- a/errors/gerror/gerror_code.go +++ b/errors/gerror/gerror_code.go @@ -27,6 +27,7 @@ const ( CodeServerBusy = 63 // Server is busy, please try again later. CodeUnknown = 64 // Unknown error. CodeResourceNotExist = 65 // Resource does not exist. + CodeInvalidRequest = 66 // Invalid request. CodeBusinessValidationFailed = 300 // Business validation failed. ) @@ -51,6 +52,7 @@ var ( CodeServerBusy: "Server Is Busy", CodeUnknown: "Unknown Error", CodeResourceNotExist: "Resource Not Exist", + CodeInvalidRequest: "Invalid Request", CodeBusinessValidationFailed: "Business Validation Failed", } ) diff --git a/errors/gerror/gerror_error.go b/errors/gerror/gerror_error.go index 0c15b0410..97e22f0cd 100644 --- a/errors/gerror/gerror_error.go +++ b/errors/gerror/gerror_error.go @@ -51,7 +51,7 @@ func (err *Error) Error() string { errStr = Message(err.code) } if err.error != nil { - if err.text != "" { + if errStr != "" { errStr += ": " } errStr += err.error.Error() diff --git a/net/ghttp/ghttp_func.go b/net/ghttp/ghttp_func.go index 9b5e09be2..2c569c0ac 100644 --- a/net/ghttp/ghttp_func.go +++ b/net/ghttp/ghttp_func.go @@ -37,12 +37,12 @@ func niceCallFunc(f func()) { // of the real error point. if err, ok := exception.(error); ok { if gerror.Code(err) != gerror.CodeNil { - panic(gerror.Wrap(err, "")) + panic(err) } else { - panic(gerror.WrapCode(gerror.CodeInternalError, err, "")) + panic(gerror.WrapCodeSkip(gerror.CodeInternalError, 1, err, "")) } } else { - panic(gerror.NewCodeSkipf(gerror.CodeInternalError, 1, "%v", exception)) + panic(gerror.NewCodeSkipf(gerror.CodeInternalError, 1, "%+v", exception)) } } } diff --git a/net/ghttp/ghttp_request_param.go b/net/ghttp/ghttp_request_param.go index 1da2af60a..e0381e854 100644 --- a/net/ghttp/ghttp_request_param.go +++ b/net/ghttp/ghttp_request_param.go @@ -13,6 +13,7 @@ import ( "github.com/gogf/gf/encoding/gjson" "github.com/gogf/gf/encoding/gurl" "github.com/gogf/gf/encoding/gxml" + "github.com/gogf/gf/errors/gerror" "github.com/gogf/gf/internal/json" "github.com/gogf/gf/internal/utils" "github.com/gogf/gf/text/gregex" @@ -299,7 +300,7 @@ func (r *Request) parseQuery() { var err error r.queryMap, err = gstr.Parse(r.URL.RawQuery) if err != nil { - panic(err) + panic(gerror.WrapCode(gerror.CodeInvalidParameter, err, "")) } } } @@ -354,12 +355,12 @@ func (r *Request) parseForm() { if gstr.Contains(contentType, "multipart/") { // multipart/form-data, multipart/mixed if err = r.ParseMultipartForm(r.Server.config.FormParsingMemory); err != nil { - panic(err) + panic(gerror.WrapCode(gerror.CodeInvalidRequest, err, "")) } } else if gstr.Contains(contentType, "form") { // application/x-www-form-urlencoded if err = r.Request.ParseForm(); err != nil { - panic(err) + panic(gerror.WrapCode(gerror.CodeInvalidRequest, err, "")) } } if len(r.PostForm) > 0 { @@ -402,7 +403,7 @@ func (r *Request) parseForm() { } if params != "" { if r.formMap, err = gstr.Parse(params); err != nil { - panic(err) + panic(gerror.WrapCode(gerror.CodeInvalidParameter, err, "")) } } } diff --git a/net/ghttp/ghttp_request_param_page.go b/net/ghttp/ghttp_request_param_page.go index 20e0c8f55..b63a290a0 100644 --- a/net/ghttp/ghttp_request_param_page.go +++ b/net/ghttp/ghttp_request_param_page.go @@ -17,13 +17,15 @@ import ( // NOTE THAT the page parameter name from client is constantly defined as gpage.DefaultPageName // for simplification and convenience. func (r *Request) GetPage(totalSize, pageSize int) *gpage.Page { - // It must has Router object attribute. + // It must have Router object attribute. if r.Router == nil { panic("Router object not found") } - url := *r.URL - urlTemplate := url.Path - uriHasPageName := false + var ( + url = *r.URL + urlTemplate = url.Path + uriHasPageName = false + ) // Check the page variable in the URI. if len(r.Router.RegNames) > 0 { for _, name := range r.Router.RegNames { @@ -39,12 +41,17 @@ func (r *Request) GetPage(totalSize, pageSize int) *gpage.Page { for i, name := range r.Router.RegNames { rule := fmt.Sprintf(`[:\*]%s|\{%s\}`, name, name) if name == gpage.DefaultPageName { - urlTemplate, _ = gregex.ReplaceString(rule, gpage.DefaultPagePlaceHolder, urlTemplate) + urlTemplate, err = gregex.ReplaceString(rule, gpage.DefaultPagePlaceHolder, urlTemplate) } else { - urlTemplate, _ = gregex.ReplaceString(rule, match[i+1], urlTemplate) + urlTemplate, err = gregex.ReplaceString(rule, match[i+1], urlTemplate) + } + if err != nil { + panic(err) } } } + } else { + panic(err) } } } diff --git a/net/ghttp/ghttp_server.go b/net/ghttp/ghttp_server.go index 041b182e7..a0153aa0c 100644 --- a/net/ghttp/ghttp_server.go +++ b/net/ghttp/ghttp_server.go @@ -107,7 +107,7 @@ func GetServer(name ...interface{}) *Server { } // Initialize the server using default configurations. if err := s.SetConfig(NewConfig()); err != nil { - panic(err) + panic(gerror.WrapCode(gerror.CodeInvalidConfiguration, err, "")) } // Record the server to internal server mapping by name. serverMapping.Set(serverName, s) diff --git a/net/ghttp/ghttp_server_handler.go b/net/ghttp/ghttp_server_handler.go index e461a9222..7803a295d 100644 --- a/net/ghttp/ghttp_server_handler.go +++ b/net/ghttp/ghttp_server_handler.go @@ -68,12 +68,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { request.Response.WriteStatus(http.StatusInternalServerError) if err, ok := exception.(error); ok { if code := gerror.Code(err); code != gerror.CodeNil { - s.handleErrorLog(gerror.Wrap(err, ""), request) + s.handleErrorLog(err, request) } else { - s.handleErrorLog(gerror.WrapCode(gerror.CodeInternalError, err, ""), request) + s.handleErrorLog(gerror.WrapCodeSkip(gerror.CodeInternalError, 1, err, ""), request) } } else { - s.handleErrorLog(gerror.NewCodef(gerror.CodeInternalError, "%v", exception), request) + s.handleErrorLog(gerror.NewCodeSkipf(gerror.CodeInternalError, 1, "%+v", exception), request) } } } diff --git a/net/ghttp/ghttp_server_router_serve.go b/net/ghttp/ghttp_server_router_serve.go index 2f36302e1..d3614312e 100644 --- a/net/ghttp/ghttp_server_router_serve.go +++ b/net/ghttp/ghttp_server_router_serve.go @@ -8,6 +8,7 @@ package ghttp import ( "fmt" + "github.com/gogf/gf/errors/gerror" "github.com/gogf/gf/internal/json" "strings" @@ -175,8 +176,8 @@ func (s *Server) searchHandlers(method, path, domain string) (parsedItems []*han parsedItemList.PushBack(parsedItem) // The middleware is inserted before the serving handler. - // If there're multiple middleware, they're inserted into the result list by their registering order. - // The middleware are also executed by their registered order. + // If there are multiple middleware, they're inserted into the result list by their registering order. + // The middleware is also executed by their registered order. case handlerTypeMiddleware: if lastMiddlewareElem == nil { lastMiddlewareElem = parsedItemList.PushFront(parsedItem) @@ -190,7 +191,7 @@ func (s *Server) searchHandlers(method, path, domain string) (parsedItems []*han parsedItemList.PushBack(parsedItem) default: - panic(fmt.Sprintf(`invalid handler type %d`, item.Type)) + panic(gerror.NewCodef(gerror.CodeInternalError, `invalid handler type %d`, item.Type)) } } } diff --git a/net/ghttp/ghttp_unit_config_test.go b/net/ghttp/ghttp_unit_config_test.go index 95ba5abfb..cd5949b97 100644 --- a/net/ghttp/ghttp_unit_config_test.go +++ b/net/ghttp/ghttp_unit_config_test.go @@ -152,7 +152,7 @@ func Test_ClientMaxBodySize_File(t *testing.T) { defer gfile.Remove(path) t.Assert( gstr.Trim(c.PostContent("/", "name=john&file=@file:"+path)), - "http: request body too large", + "Invalid Request: http: request body too large", ) }) }