From ad74d7b9359f942c17bd9b4f66023ba2431da4cb Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Mon, 11 May 2020 11:11:27 +0300 Subject: [PATCH] error handlers per party: log those routes too and all test cases paased need cleanup though Former-commit-id: 6b0c18e38b58af2388015c7cf1af9cc43d7d35d3 --- core/router/api_builder.go | 45 +++++++++++++++++--------------------- core/router/handler.go | 15 ++++++++++++- core/router/route.go | 17 +++++++++----- core/router/status_test.go | 23 +++++++++++-------- hero/dependency_source.go | 5 ++++- macro/handler/handler.go | 10 +++++++-- 6 files changed, 71 insertions(+), 44 deletions(-) diff --git a/core/router/api_builder.go b/core/router/api_builder.go index d9052d6e..86ff1b0f 100644 --- a/core/router/api_builder.go +++ b/core/router/api_builder.go @@ -981,23 +981,11 @@ func (api *APIBuilder) Favicon(favPath string, requestPath ...string) *Route { func (api *APIBuilder) OnErrorCode(statusCode int, handlers ...context.Handler) { // TODO: think a stable way for that and document it so end-developers // not be suprised. Many feature requests in the past asked for that per-party error handlers. + api.handle(statusCode, "", "/", handlers...) + if api.relativePath != "/" { api.handle(statusCode, "", "/{tail:path}", handlers...) } - - api.handle(statusCode, "", "/", handlers...) - - // if api.relativePath != "/" /* root is OK, no need to wildcard, see handler.go */ && - // !strings.HasSuffix(api.relativePath, "}") /* and not /users/{id:int} */ { - // // We need to register the /users and the /users/{tail:path}, - // api.handle(statusCode, "", "/{tail:path}", handlers...) - // } - - // if strings.HasSuffix(api.relativePath, "/") { - // api.handle(statusCode, "", "/", handlers...) - // } - - // api.handle(statusCode, "", "/{tail:path}", handlers...) } // OnAnyErrorCode registers a handler which called when error status code written. @@ -1058,25 +1046,32 @@ func getCaller() (string, int) { wd, _ := os.Getwd() for { frame, more := frames.Next() - file := frame.File + file := filepath.ToSlash(frame.File) - if strings.Contains(file, "src/testing/testing.go") { + if !strings.Contains(file, "_test.go") { + if strings.Contains(file, "/kataras/iris") && !strings.Contains(file, "kataras/iris/_examples") && !strings.Contains(file, "iris-contrib/examples") { + if !more { + break + } + continue + } + } + + if strings.Contains(file, "go/src/runtime/") { + if !more { + break + } continue } - if !strings.Contains(file, "/kataras/iris") || - strings.Contains(file, "_examples") || - strings.Contains(file, "examples") { - if relFile, err := filepath.Rel(wd, file); err == nil { + if relFile, err := filepath.Rel(wd, file); err == nil { + if !strings.HasPrefix(relFile, "..") { + // Only if it's relative to this path, not parent. file = "./" + relFile } - - return file, frame.Line } - if !more { - break - } + return file, frame.Line } return "???", 0 diff --git a/core/router/handler.go b/core/router/handler.go index af69b017..5dae2129 100644 --- a/core/router/handler.go +++ b/core/router/handler.go @@ -216,7 +216,7 @@ func (h *routerHandler) Build(provider RoutesProvider) error { routes []*Route } - allMethods := append(AllMethods, MethodNone) + allMethods := append(AllMethods, []string{MethodNone, ""}...) methodRoutes := make([]MethodRoutes, 0, len(allMethods)) for _, method := range allMethods { @@ -246,6 +246,9 @@ func (h *routerHandler) Build(provider RoutesProvider) error { fmt.Fprint(logger.Printer, ", ") } } + if m.method == "" { + m.method = "ERROR" + } fmt.Fprintf(logger.Printer, "%d ", len(m.routes)) pio.WriteRich(logger.Printer, m.method, traceMethodColor(m.method)) } @@ -459,6 +462,16 @@ func (h *routerHandler) FireErrorCode(ctx context.Context) { } if n != nil { + // Note: handlers can contain macro filters here, + // they are registered as error handlers, see macro/handler.go#42. + + // fmt.Println("Error Handlers") + // for _, h := range n.Handlers { + + // f, l := context.HandlerFileLine(h) + // fmt.Printf("%s: %s:%d\n", ctx.Path(), f, l) + // } + // fire this http status code's error handlers chain. // ctx.StopExecution() // not uncomment this, is here to remember why to. diff --git a/core/router/route.go b/core/router/route.go index 0b18eabd..cefbfa12 100644 --- a/core/router/route.go +++ b/core/router/route.go @@ -386,7 +386,7 @@ func traceMethodColor(method string) int { return color } - return pio.Black + return 131 // for error handlers, of "ERROR [%STATUSCODE]" } // Trace prints some debug info about the Route to the "w". @@ -397,13 +397,18 @@ func traceMethodColor(method string) int { // * @second_handler ... // If route and handler line:number locations are equal then the second is ignored. func (r *Route) Trace(w io.Writer) { + method := r.Method + if method == "" { + method = fmt.Sprintf("%d", r.StatusCode) + } + // Color the method. - color := traceMethodColor(r.Method) + color := traceMethodColor(method) // @method: @path - // space := strings.Repeat(" ", len(http.MethodConnect)-len(r.Method)) - // s := fmt.Sprintf("%s: %s", pio.Rich(r.Method, color), path) - pio.WriteRich(w, r.Method, color) + // space := strings.Repeat(" ", len(http.MethodConnect)-len(method)) + // s := fmt.Sprintf("%s: %s", pio.Rich(method, color), path) + pio.WriteRich(w, method, color) path := r.Tmpl().Src if path == "" { @@ -415,7 +420,7 @@ func (r *Route) Trace(w io.Writer) { // (@description) description := r.Description if description == "" { - if r.Method == MethodNone { + if method == MethodNone { description = "offline" } diff --git a/core/router/status_test.go b/core/router/status_test.go index f2f9a7b4..11e27cd4 100644 --- a/core/router/status_test.go +++ b/core/router/status_test.go @@ -89,27 +89,30 @@ func TestPartyOnErrorCode(t *testing.T) { app.Get("/path", h) - h := func(ctx iris.Context) { ctx.WriteString(ctx.Path()) } usersResponse := "users: method allowed" - users := app.Party("/users") users.OnErrorCode(iris.StatusMethodNotAllowed, func(ctx iris.Context) { ctx.WriteString(usersResponse) }) users.Get("/", h) + write400 := func(ctx iris.Context) { ctx.StatusCode(iris.StatusBadRequest) } // test setting the error code from a handler. - users.Get("/badrequest", func(ctx iris.Context) { ctx.StatusCode(iris.StatusBadRequest) }) + users.Get("/badrequest", write400) usersuserResponse := "users:user: method allowed" user := users.Party("/{id:int}") user.OnErrorCode(iris.StatusMethodNotAllowed, func(ctx iris.Context) { ctx.WriteString(usersuserResponse) }) - // usersuserNotFoundResponse := "users:user: not found" - // user.OnErrorCode(iris.StatusNotFound, func(ctx iris.Context) { - // ctx.WriteString(usersuserNotFoundResponse) - // }) + usersuserNotFoundResponse := "users:user: not found" + user.OnErrorCode(iris.StatusNotFound, func(ctx iris.Context) { + ctx.WriteString(usersuserNotFoundResponse) + }) user.Get("/", h) + user.Get("/ab/badrequest", write400) + + friends := users.Party("/friends") + friends.Get("/{id:int}", h) e := httptest.New(t, app) @@ -125,10 +128,12 @@ func TestPartyOnErrorCode(t *testing.T) { e.GET("/users/42").Expect().Status(iris.StatusOK). Body().Equal("/users/42") - // e.GET("/users/ab").Expect().Status(iris.StatusNotFound).Body().Equal(usersuserNotFoundResponse) + e.GET("/users/ab").Expect().Status(iris.StatusNotFound).Body().Equal(usersuserNotFoundResponse) + // inherit the parent. + e.GET("/users/42/friends/dsa").Expect().Status(iris.StatusNotFound).Body().Equal(usersuserNotFoundResponse) // if not registered to the party, then the root is taking action. - e.GET("/users/ab/cd").Expect().Status(iris.StatusNotFound).Body().Equal(globalNotFoundResponse) + e.GET("/users/42/ab/badrequest").Expect().Status(iris.StatusBadRequest).Body().Equal(http.StatusText(iris.StatusBadRequest)) // if not registered to the party, and not in root, then just write the status text (fallback behavior) e.GET("/users/badrequest").Expect().Status(iris.StatusBadRequest). diff --git a/hero/dependency_source.go b/hero/dependency_source.go index 87f5aff9..579427a2 100644 --- a/hero/dependency_source.go +++ b/hero/dependency_source.go @@ -41,7 +41,10 @@ func newSource(fn reflect.Value) Source { wd, _ := os.Getwd() if relFile, err := filepath.Rel(wd, callerFileName); err == nil { - callerFileName = "./" + relFile + if !strings.HasPrefix(relFile, "..") { + // Only if it's relative to this path, not parent. + callerFileName = "./" + relFile + } } return Source{ diff --git a/macro/handler/handler.go b/macro/handler/handler.go index ed2013b0..f41a17b0 100644 --- a/macro/handler/handler.go +++ b/macro/handler/handler.go @@ -39,11 +39,17 @@ func MakeHandler(tmpl macro.Template) context.Handler { return func(ctx context.Context) { if !filter(ctx) { - ctx.StopExecution() + if ctx.GetCurrentRoute().StatusErrorCode() == ctx.GetStatusCode() { + ctx.Next() + } else { + ctx.StopExecution() + } + return } - // if all passed, just continue. + // if all passed or the next is the registered error handler to handle this status code, + // just continue. ctx.Next() } }