[FEATURE REQUEST] MVC - More Elegent OnErrorCode registration? #1595
Comments
|
Hello @AlbinoGeek, This is relatively to : Question: MVC Custom Error Handler? (#1244). For users who read this post, you can already do this to handle the errors output of a controller method: mvcApp := mvc.New(app)
mvcApp.HandleError(func(ctx iris.Context, err error) {
ctx.HTML(fmt.Sprintf("<b>%s</b>", err.Error()))
})
mvcApp.Handle(new(myController))OR inside a controller: type myController struct {
}
// overriddes the mvcApp.HandleError function for this controller.
func (c *myController) HandleError(ctx iris.Context, err error) {
ctx.HTML(fmt.Sprintf("<i>%s</i>", err.Error()))
}
func (c *myController) Get() error {
return errors.New("test error")
} So the suggestion of yours is to handle HTTP errors inside the controller itself? I mean it can be possible now that we support We need to think and discuss of it |
|
Well, does the current code allow multiple If it does, perhaps that is not much different than this case. Also, in these examples, if you returned Otherwise, they could be stacked like Middleware if you really wanted multiple error handlers (I could see use for this), such as Example of stacking use case:
in SEO-inspired URL environments, e.g: Although, yes, technically this could also be solved using a controller pre-handler ( That makes me wonder, how does |
|
On further thought, I am leaning towards the implementation: func (c *PublicController) OnError(code iris.StatusCode, [ctx iris.Context]) mvc.Result {
select code {
case iris.StatusNotFound:
return mvc.View{Name: "404.html"}
default:
return mvc.View{Name: "500.html"}
}
}Because then it doubles as a fail-through that I can call from my other routes, such as in the func (c *UserController) GetBy(userid uint64, ctx iris.Context) mvc.Result {
user, err := c.userRepository.FindByID(userid)
if _, notFound := err.(db.NoRecordError); notFound {
// follows DRY on handling errors in one place in the controller
// v.s. literally in every GetBy, PostBy, etc etc.
return c.OnError(iris.NotFound, ctx)
}
// ... existing code
} |
Yes, they run as middlewares, chain of handlers with @AlbinoGeek you don't need to call a By using custom mvc.ResultYou can just create an The below creates a custom package main
import (
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/mvc"
)
func main() {
app := iris.New()
app.RegisterView(iris.HTML("./views", ".html"))
m := mvc.New(app)
m.Handle(new(controller))
app.Listen(":8080")
}
type errorResponse struct {
Code int
Message string
}
/*
// Note: if a struct implements the standard go error, so it's an error
// and its Error() is not empty, then its text will be rendered instead,
// override any Dispatch method.
func (e errorResponse) Error() string {
return e.Message
}
*/
// implements mvc.Result.
func (e errorResponse) Dispatch(ctx iris.Context) {
// If u want to use mvc.Result on any method without an output return value
// go for it:
//
view := mvc.View{Code: e.Code, Data: e} // use Code and Message as the template data.
// TIP: convert e.Code to string and fire the
// desired vie automatically for known errors that can be rendered
// through existing view files.
// view.Name = fmt.Sprintf("%d", e.Code)
// and view.Dispath(ctx)
switch e.Code {
case iris.StatusNotFound:
view.Name = "404"
default:
view.Name = "500"
}
view.Dispatch(ctx)
// Otherwise use ctx methods:
//
// if r.Code > 0 {
// ctx.StatusCode(r.Code)
// }
// switch e.Code {
// case iris.StatusNotFound:
// // use Code and Message as the template data.
// ctx.View("404.html", e)
// default:
// ctx.View("500.html", e)
// }
}
type controller struct{}
type user struct {
ID uint64 `json:"id"`
}
func (c *controller) GetBy(userid uint64) mvc.Result {
if userid != 1 {
return errorResponse{
Code: iris.StatusNotFound,
Message: "User Not Found",
}
}
return mvc.Response{
Object: user{ID: userid},
}
}By Hijacking the response type and valuepackage main
import (
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/mvc"
)
func main() {
app := iris.New()
app.RegisterView(iris.HTML("./views", ".html"))
// Hijack each output value of a method (can be used per-party too).
app.ConfigureContainer().
UseResultHandler(func(next iris.ResultHandler) iris.ResultHandler {
return func(ctx iris.Context, v interface{}) error {
switch val := v.(type) {
case errorResponse:
return next(ctx, errorView(val))
default:
return next(ctx, v)
}
}
})
m := mvc.New(app)
m.Handle(new(controller))
app.Listen(":8080")
}
func errorView(e errorResponse) mvc.Result {
switch e.Code {
case iris.StatusNotFound:
return mvc.View{Code: e.Code, Name: "404.html", Data: e}
default:
return mvc.View{Code: e.Code, Name: "500.html", Data: e}
}
}
type errorResponse struct {
Code int
Message string
}
type controller struct{}
type user struct {
ID uint64 `json:"id"`
}
func (c *controller) GetBy(userid uint64) interface{} {
if userid != 1 {
return errorResponse{
Code: iris.StatusNotFound,
Message: "User Not Found",
}
}
return user{ID: userid}
}By implementing PreflightResult (my favorite method)And optionally will implement the PreflightResult interface if you want to do more, example: (An package main
import (
"fmt"
"time"
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/mvc"
)
func main() {
app := iris.New()
app.RegisterView(iris.HTML("./views", ".html"))
m := mvc.New(app)
m.Handle(new(controller))
app.Listen(":8080")
}
type controller struct{}
// Generic response type for JSON results.
type response struct {
ID uint64 `json:"id,omitempty"`
Data interface{} `json:"data,omitempty"` // {data: result } on fetch actions.
Code int `json:"code,omitempty"`
Message string `json:"message,omitempty"`
Timestamp int64 `json:"timestamp,omitempty"`
}
func (r response) Preflight(ctx iris.Context) error {
if r.ID > 0 {
r.Timestamp = time.Now().Unix()
}
if code := r.Code; code > 0 {
// You can call ctx.View or mvc.Vew{...}.Dispatch
// to render HTML on Code != 200
// but in order to not proceed with the response resulting
// as JSON you MUST return the iris.ErrStopExecution error.
// Example:
if code != 200 {
mvc.View{
/* calls the ctx.StatusCode */
Code: code,
/* use any r.Data as the template data
OR the whole "response" as its data. */
Data: r,
/* automatically pick the template per error (just for the sake of the example) */
Name: fmt.Sprintf("%d", code),
}.Dispatch(ctx)
return iris.ErrStopExecution
}
ctx.StatusCode(r.Code)
}
return nil
}
type user struct {
ID uint64 `json:"id"`
}
func (c *controller) GetBy(userid uint64) response {
if userid != 1 {
return response{
Code: iris.StatusNotFound,
Message: "User Not Found",
}
}
return response{
ID: userid,
Data: user{ID: userid},
}
}You can use that c := app.ConfigureContainer()
c.Get("/{id:uint64}", getUserByID)func getUserByID(id uint64) response {
if userid != 1 {
return response{
Code: iris.StatusNotFound,
Message: "User Not Found",
}
}
return response{
ID: userid,
Data: user{ID: userid},
}
}You can still use |
|
I do apologize in my delay on updating this issue, as you have given such a wonderfully detailed response. I have been working on implementing each of these suggestions and figuring out which one matches my requirements, to produce what would amount to a documented response, with some examples of usage that could be committed to the MVC You put up some very valid questions in the above, and I'd love to give these each the attention they need! I should be able to over this weekend, to give this thread a proper response. |
|
Alright, perhaps I've gone about this wrong, but it would appear the above only really works for JSON formatted data. When I attempted to implement the Preflight Methodtype PublicResponse mvc.View
func (r PublicResponse) Preflight(ctx iris.Context) error {
if r.Code == 0 { // for some reason, Code is always 0
r.Code = 200
}
ctx.StatusCode(r.Code)
if r.Code != 200 {
mvc.View{ // unnecessary copy perhaps? but no dispatch if direct inherit
Name: fmt.Sprintf("%d.html", r.Code),
Code: r.Code,
Data: r.Data,
Err: r.Err,
Layout: r.Layout,
}.Dispatch(ctx)
return iris.ErrStopExecution
}
return nil
}
func (c *PublicController) Get() PublicResponse {
return PublicResponse{
Name: "index.html",
}
}Result{
"Name": "index.html",
"Layout": "",
"Data": null, /* did not respect ViewData */
"Code": 0, /* did not even fire the Preflight, as code would have been 200 then */
"Err": null
}And it doesn't handle 404s, 500s, etc. Custom mvc.Resulttype PublicResponse mvc.View
func (r PublicResponse) Dispatch(ctx iris.Context) {
if r.Code == 0 { // correct for writeHeader 0 panic
r.Code = 200
}
if r.Code != 200 { // catch and redirect error codes
r.Name = fmt.Sprintf("%d.html", r.Code)
}
ctx.StatusCode(r.Code)
mvc.View{
Name: r.Name,
Code: r.Code,
Data: r.Data,
Err: r.Err,
Layout: r.Layout,
}.Dispatch(ctx)
}
func (c *PublicController) Get() mvc.Result {
return PublicResponse{
Name: "index.html",
}
}ResultIt works! But, it doesn't handle 404s, 500s, etc. Hijacking Response Type and ValueNot attempted, because I couldn't see a way to attach it to an MVC Controller only. HandleErrorfunc (c *PublicController) HandleError() error {
return fmt.Errorf("Testing, 123!")
}404s, 500s, etc did not trigger this code, so I'm still not sure what Basically, my goal was to manage all errors in one place per controller. Something that would avoid the explicit running of a giant list of Currently, because of signatures, I can't even use the same function to handle regular errors as method errors, because:
Not to mention the requirement to both func (c *PublicController) BeforeActivation(b mvc.BeforeActivation) {
// request ID supplement
b.Router().Use(requestid.New())
b.Router().UseError(requestid.New())
// require basicauth
auth := basicauth.New(basicauth.Config{
Users: viper.GetStringMapString("AuthorizedUsers"),
Realm: "Authorization Required", // defaults to "Authorization Required"
Expires: time.Duration(30) * time.Minute,
})
b.Router().Use(auth)
b.Router().UseError(auth)
// cache pages
b.Router().Use(cache.Handler(viper.Get("StaticPageCacheTime").(time.Duration)))
// compress pages
b.Router().Use(iris.Compression)
b.Router().UseError(iris.Compression)
// load templates used by this subdomain
b.Router().RegisterView(iris.HTML(viper.GetString("TemplatesDir"), ".html").
Layout("layouts/base.html").
Reload(viper.GetBool("DevMode")))
// make sure ViewData is populated as expected
b.Router().Use(populateViewData)
b.Router().UseError(populateViewData)
// 404 Handler - this isn't elegant if I want to implement multiple error handlers per Controller
b.Router().OnErrorCode(iris.StatusNotFound, func(ctx iris.Context) {
ctx.ViewData("title", fmt.Sprintf("%s - Not Found", viper.GetString("SiteTitle")))
ctx.StatusCode(iris.StatusNotFound)
if err := ctx.View("404.html"); err != nil {
ctx.StatusCode(iris.StatusInternalServerError)
ctx.Writef(err.Error())
}
})
} |
|
Hello @AlbinoGeek, no worries about the delayed response, you do well thinking one of those. Let me explain
The
First of all no need to make another copy of return PublicResponse{
Name: "index.html",
}You never gave it a
You do not need to register middlewares in your MVC application when they already registered. The |
|
@AlbinoGeek If you really need to handle http errors inside the Controller I can implement a |
|
In the interest for security, easy maintainability, etc-- I had registered middleware specifically on the Controller because I do not want Compression on the That being said, could I be using And yes, the issue with |
package main
import (
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/mvc"
)
func main() {
app := iris.New()
app.Logger().SetLevel("debug")
app.RegisterView(iris.HTML("./views", ".html"))
m := mvc.New(app)
m.Handle(new(controller))
app.Listen(":8080")
}
type controller struct {
}
func (c *controller) Get() string {
return "Hello!"
}
// The input parameter of mvc.Code is optional but a good practise to follow.
// You could register a Context and get its error code through ctx.GetStatusCode().
//
// This can accept dependencies and output values like any other Controller Method,
// however be careful if your registered dependencies depend only on succesful(200...) requests.
//
// Also, take a note that if you register more than one controller.HandleHTTPError
// in the same Party, you need to use the RouteOverlap feature as shown
// in the "authenticated-controller" example, and a dependency on
// a controller's field (or method's input argument) is required
// to select which, between those two controllers, is responsible
// to handle http errors.
func (c *controller) HandleHTTPError(statusCode mvc.Code) mvc.View {
code := int(statusCode) // cast it to int.
view := mvc.View{
Code: code,
Name: "unexpected-error.html",
}
switch code {
case 404:
view.Name = "404.html"
// [...]
case 500:
view.Name = "500.html"
}
return view
}
@AlbinoGeek does something like that suits your needs? (I've managed to do it without huge coblexity or breaking changes)
|
|
This is awesome! This code change has removed 206 mostly duplicate lines of code from my models. I especially appreciated that when a specified HTML file was not found, the code was served up with no content (allowing the browser to render its best approximation for the respective error supplied), which means I can fall-through to The only thing I would ask, is that to be consistent with other APIs, is there any reason your example returned a func (c *PublicController) HandleHTTPError(ctx iris.Context, statusCode mvc.Code) mvc.Result {
view := mvc.View{
Code: int(statusCode),
}
switch view.Code {
case 400, 403, 404, 410, 418, 451, 502, 504, 511:
view.Name = fmt.Sprintf("error-%d.html", view.Code)
case 429, 503:
ctx.Header("Retry-After", c.whenShouldRetry(ctx))
default:
view.Name = "error-500.html"
}
return view
}For me, I'd call this resolved and then some. |
|
@AlbinoGeek Thanks, I am glad that helped!!
No, it's the same. The |
Summary
Chances are I've just missed something in the documentation, but my current way of registering error handlers in an MVC Controller is rather... inelegant. I basically took my existing non-MVC code and dropped it directly into a Controller file.
Code Currently
Suggestions
Rely on Golang 1.13+
errors.Ashttps://blog.golang.org/go1.13-errors
Support Golang <1.13 without
errors.AsThe text was updated successfully, but these errors were encountered: