From 6e7224e306f16640ed249b6bfab5aed203982e1d Mon Sep 17 00:00:00 2001 From: John Date: Tue, 28 Apr 2020 15:04:07 +0800 Subject: [PATCH] improve error handling for gconv.Struct/ghttp.Server; add NewSkip/NewfSkip function for package gerror --- .example/other/test.go | 104 ++++++++++++++++++++----- errors/gerror/gerror.go | 29 +++++++ errors/gerror/gerror_error.go | 10 --- errors/gerror/gerror_stack.go | 14 +++- net/ghttp/ghttp_func.go | 11 ++- net/ghttp/ghttp_request_middleware.go | 9 ++- net/ghttp/ghttp_request_param.go | 2 +- net/ghttp/ghttp_server_log.go | 28 +++---- util/gconv/gconv_struct.go | 13 +++- util/gconv/gconv_z_unit_struct_test.go | 19 +++++ 10 files changed, 180 insertions(+), 59 deletions(-) diff --git a/.example/other/test.go b/.example/other/test.go index 480b68fc2..45aedba3b 100644 --- a/.example/other/test.go +++ b/.example/other/test.go @@ -1,25 +1,93 @@ package main import ( + "github.com/gogf/gf/errors/gerror" "github.com/gogf/gf/frame/g" - "github.com/gogf/gf/util/gconv" + "github.com/gogf/gf/net/ghttp" ) -func main() { - type SaveReq1 struct { - Id uint - Tags string - } - type SaveReq2 struct { - Id uint - Tags []string - } - r1 := SaveReq1{ - Id: 1, - Tags: "ac", - } - var r2 *SaveReq2 - err := gconv.Struct(r1, &r2) - g.Dump(err) - g.Dump(r2) +// ListsInput ListsInput +type ListsInput struct { + Page int `v:"min:1#page不能小于1"` + Limit int `v:"between:10,100#limit必须是10-100之间的整数"` + Search +} + +type ListRequest struct { + ListsInput +} + +// Search Search +type Search struct { + GoodsName string + GoodsSource string + FreeShipping string + MarketPrice struct { + Min int + Max int + IsInclude bool + } + GuidePrice struct { + Min int + Max int + IsInclude bool + } + AgreementPrice struct { + Min int + Max int + IsInclude bool + } + NormalProfitMargin struct { + Min int + Max int + IsInclude bool + } + ActivityProfitMargin struct { + Min int + Max int + IsInclude bool + } + Sales struct { + Cycle string + Quantity struct { + Min int + Max int + IsInclude bool + } + } + SalesReturn struct { + Cycle string + Quantity struct { + Min int + Max int + IsInclude bool + } + } + ChooseGoods struct { + Cycle string + Quantity struct { + Min int + Max int + IsInclude bool + } + } +} + +// Lists Lists +func Lists(r *ghttp.Request) { + var params *ListRequest + if err := r.Parse(¶ms); err != nil { + r.Response.WriteExit(gerror.Stack(err)) + } + + r.Response.Write(params) +} + +func main() { + s := g.Server() + s.Group("/", func(group *ghttp.RouterGroup) { + group.POST("/test", Lists) + }) + s.SetPort(8199) + s.Run() } diff --git a/errors/gerror/gerror.go b/errors/gerror/gerror.go index 11c6e55ba..d62d41380 100644 --- a/errors/gerror/gerror.go +++ b/errors/gerror/gerror.go @@ -5,6 +5,9 @@ // You can obtain one at https://github.com/gogf/gf. // Package errors provides simple functions to manipulate errors. +// +// Very note that, this package is quite a base package, which should not import extra +// packages except standard packages, to avoid cycle imports. package gerror import ( @@ -13,11 +16,13 @@ import ( // ApiStack is the interface for Stack feature. type ApiStack interface { + Error() string // It should be en error. Stack() string } // ApiCause is the interface for Cause feature. type ApiCause interface { + Error() string // It should be en error. Cause() error } @@ -32,6 +37,18 @@ func New(text string) error { } } +// NewSkip creates and returns an error which is formatted from given text. +// The parameter specifies the stack callers skipped amount. +func NewSkip(skip int, text string) error { + if text == "" { + return nil + } + return &Error{ + stack: callers(skip), + text: text, + } +} + // Newf returns an error that formats as the given format and args. func Newf(format string, args ...interface{}) error { if format == "" { @@ -43,6 +60,18 @@ func Newf(format string, args ...interface{}) error { } } +// NewfSkip returns an error that formats as the given format and args. +// The parameter specifies the stack callers skipped amount. +func NewfSkip(skip int, format string, args ...interface{}) error { + if format == "" { + return nil + } + return &Error{ + stack: callers(skip), + text: fmt.Sprintf(format, args...), + } +} + // Wrap wraps error with text. // It returns nil if given err is nil. func Wrap(err error, text string) error { diff --git a/errors/gerror/gerror_error.go b/errors/gerror/gerror_error.go index 43a99a172..b0cf7b539 100644 --- a/errors/gerror/gerror_error.go +++ b/errors/gerror/gerror_error.go @@ -9,7 +9,6 @@ package gerror import ( "bytes" "fmt" - "github.com/gogf/gf/internal/intlog" "io" "runtime" "strings" @@ -131,15 +130,6 @@ func formatSubStack(st stack, buffer *bytes.Buffer) { if strings.Contains(file, gFILTER_KEY) { continue } - // Avoid GF stacks if not in GF development. - if !intlog.IsEnabled() { - if strings.Contains(file, "github.com/gogf/gf/") { - continue - } - if strings.Contains(file, "github.com/gogf/gf@") { - continue - } - } // Avoid stack string like "" if strings.Contains(file, "<") { continue diff --git a/errors/gerror/gerror_stack.go b/errors/gerror/gerror_stack.go index 5a0d5239b..ce64255aa 100644 --- a/errors/gerror/gerror_stack.go +++ b/errors/gerror/gerror_stack.go @@ -15,8 +15,14 @@ const ( gMAX_STACK_DEPTH = 32 ) -func callers() stack { - var pcs [gMAX_STACK_DEPTH]uintptr - n := runtime.Callers(3, pcs[:]) - return pcs[0:n] +// callers returns the stack callers. +func callers(skip ...int) stack { + var ( + pcs [gMAX_STACK_DEPTH]uintptr + n = 3 + ) + if len(skip) > 0 { + n += skip[0] + } + return pcs[:runtime.Callers(n, pcs[:])] } diff --git a/net/ghttp/ghttp_func.go b/net/ghttp/ghttp_func.go index 3a55296fb..a48f0fa7f 100644 --- a/net/ghttp/ghttp_func.go +++ b/net/ghttp/ghttp_func.go @@ -7,6 +7,7 @@ package ghttp import ( + "github.com/gogf/gf/errors/gerror" "strings" "github.com/gogf/gf/encoding/gurl" @@ -54,12 +55,16 @@ func BuildParams(params interface{}, noUrlEncode ...bool) (encodedParamStr strin // niceCallFunc calls function with exception capture logic. func niceCallFunc(f func()) { defer func() { - if err := recover(); err != nil { - switch err { + if e := recover(); e != nil { + switch e { case gEXCEPTION_EXIT, gEXCEPTION_EXIT_ALL: return default: - panic(err) + if _, ok := e.(gerror.ApiStack); ok { + panic(e) + } else { + panic(gerror.NewfSkip(1, "%v", e)) + } } } }() diff --git a/net/ghttp/ghttp_request_middleware.go b/net/ghttp/ghttp_request_middleware.go index 495a6431f..8e751b329 100644 --- a/net/ghttp/ghttp_request_middleware.go +++ b/net/ghttp/ghttp_request_middleware.go @@ -7,11 +7,10 @@ package ghttp import ( + "github.com/gogf/gf/errors/gerror" "net/http" "reflect" - "github.com/gogf/gf/errors/gerror" - "github.com/gogf/gf/util/gutil" ) @@ -122,7 +121,11 @@ func (m *Middleware) Next() { loop = false } }, func(exception interface{}) { - m.request.error = gerror.Newf("%v", exception) + if e, ok := exception.(gerror.ApiStack); ok { + m.request.error = e.(error) + } else { + m.request.error = gerror.NewfSkip(1, "%v", exception) + } m.request.Response.WriteStatus(http.StatusInternalServerError, exception) loop = false }) diff --git a/net/ghttp/ghttp_request_param.go b/net/ghttp/ghttp_request_param.go index 630fec7df..5dafe3818 100644 --- a/net/ghttp/ghttp_request_param.go +++ b/net/ghttp/ghttp_request_param.go @@ -280,7 +280,7 @@ func (r *Request) parseForm() { // Only allow chars of: '\w', '[', ']', '-'. if !gregex.IsMatchString(`^[\w\-\[\]]+$`, name) && len(r.PostForm) == 1 { // It might be JSON/XML content. - if s := name + strings.Join(values, " "); len(s) > 0 { + if s := gstr.Trim(name + strings.Join(values, " ")); len(s) > 0 { if s[0] == '{' && s[len(s)-1] == '}' || s[0] == '<' && s[len(s)-1] == '>' { r.bodyContent = gconv.UnsafeStrToBytes(s) params = "" diff --git a/net/ghttp/ghttp_server_log.go b/net/ghttp/ghttp_server_log.go index 7d743ab8e..92098a790 100644 --- a/net/ghttp/ghttp_server_log.go +++ b/net/ghttp/ghttp_server_log.go @@ -12,10 +12,6 @@ import ( "github.com/gogf/gf/os/glog" ) -const ( - gPATH_FILTER_KEY = "github.com/gogf/gf/" -) - // Logger returns the logger of the server. func (s *Server) Logger() *glog.Logger { return s.config.Logger @@ -54,22 +50,22 @@ func (s *Server) handleErrorLog(err error, r *Request) { scheme = "https" } content := fmt.Sprintf( - `%d, "%s %s %s %s %s" %.3f, %s, "%s", "%s"`, + `%d "%s %s %s %s %s" %.3f, %s, "%s", "%s"`, r.Response.Status, r.Method, scheme, r.Host, r.URL.String(), r.Proto, float64(r.LeaveTime-r.EnterTime)/1000, r.GetClientIp(), r.Referer(), r.UserAgent(), ) - if stack := gerror.Stack(err); stack != "" { - content += "\nStack:\n" + stack - s.config.Logger.File(s.config.ErrorLogPattern). - Stack(false). - Stdout(s.config.LogStdout). - Error(content) - return + if s.config.ErrorStack { + if stack := gerror.Stack(err); stack != "" { + content += "\nStack:\n" + stack + } else { + content += ", " + err.Error() + } + } else { + content += ", " + err.Error() } - s.Logger().File(s.config.ErrorLogPattern). - Stack(s.config.ErrorStack). - StackWithFilter(gPATH_FILTER_KEY). + s.config.Logger. + File(s.config.ErrorLogPattern). Stdout(s.config.LogStdout). - Error(content) + Print(content) } diff --git a/util/gconv/gconv_struct.go b/util/gconv/gconv_struct.go index 7f57f0735..4cc12179d 100644 --- a/util/gconv/gconv_struct.go +++ b/util/gconv/gconv_struct.go @@ -9,6 +9,7 @@ package gconv import ( "errors" "fmt" + "github.com/gogf/gf/errors/gerror" "github.com/gogf/gf/internal/empty" "reflect" "regexp" @@ -31,8 +32,8 @@ var ( ) // Struct maps the params key-value pairs to the corresponding struct object's properties. -// The third parameter is unnecessary, indicating the mapping rules between the custom key name -// and the attribute name(case sensitive). +// The third parameter is unnecessary, indicating the mapping rules between the +// custom key name and the attribute name(case sensitive). // // Note: // 1. The can be any type of map/struct, usually a map. @@ -42,14 +43,18 @@ var ( // It will automatically convert the first letter of the key to uppercase // in mapping procedure to do the matching. // It ignores the map key, if it does not match. -func Struct(params interface{}, pointer interface{}, mapping ...map[string]string) error { +func Struct(params interface{}, pointer interface{}, mapping ...map[string]string) (err error) { if params == nil { return errors.New("params cannot be nil") } if pointer == nil { return errors.New("object pointer cannot be nil") } - + defer func() { + if e := recover(); e != nil { + err = gerror.NewfSkip(1, "%v", e) + } + }() // paramsMap is the map[string]interface{} type variable for params. paramsMap := Map(params) if paramsMap == nil { diff --git a/util/gconv/gconv_z_unit_struct_test.go b/util/gconv/gconv_z_unit_struct_test.go index d5e65c2c6..a16dd2be3 100644 --- a/util/gconv/gconv_z_unit_struct_test.go +++ b/util/gconv/gconv_z_unit_struct_test.go @@ -780,3 +780,22 @@ func Test_Struct_Complex(t *testing.T) { t.Assert(model.Data.ResultDetail.CurrentReportDetail.LoansProductCount, "8") }) } + +func Test_Struct_CatchPanic(t *testing.T) { + gtest.C(t, func(t *gtest.T) { + type Score struct { + Name string + Result int + } + type User struct { + Score + } + + user := new(User) + scores := map[string]interface{}{ + "Score": 1, + } + err := gconv.Struct(scores, user) + t.AssertNE(err, nil) + }) +}