Error handling

[azr]

Hello there !

I like goji, it's all simple and light and I have a question : How would you do simple error handling ?
Like, may be in this post : http://blog.golang.org/error-handling-and-go ?
Having some handler funcs that look like this :

func viewStuff(w http.ResponseWriter, r *http.Request) *appError

With appError looking like this :

type appError struct {
    Error   error
    Message string
    Code    int
}

So then in the handler I could :

if err := datastore.Get(c, key, record); err != nil {
    return &appError{err, "Record not found", 404}
}

Thanks !

[zenazn]

You can actually use the code in that blog post verbatim—Goji fully supports net/http's interfaces.

Perhaps the thing that's tripping you up is the fact that you can't use viewStuff directly in Goji. Just like in net/http, you have to wrap the handler in some way. In that blog post's case, they return a type that implements http.Handler.

[azr]

Okay, you are right I was tripping on that, I get it now, thanks for your answer ! :)

Do you think this is a good idea btw ??

[elithrar]

You should (IMO) return something like an (int, string, err) or just an (int, err) as it's a little (barely, mind you!) nicer on the garbage collector. I implemented this pattern a long while ago on an app and like it a lot.

Note that if you wanted to return some more context with your error you can return 404, fmt.Errorf("Record not found: %q", err) instead.

[zenazn]

I personally don't find the burden of calling http.Error that great—I tend to have relatively small http.Handlers which only need to deal with a small number of error values. I keep most of my business logic (etc.) in "ordinary" Go code, which respects the normal error returning conventions and have unconstrained interfaces.

But YMMV; I haven't had my 10,000 hours with Goji / net/http yet and so there's no reason your opinion isn't as good as mine here.

[elithrar]

My biggest issue is when you have something like:

func SomeHandler(c web.C, w http.ResponseWriter, r *http.Request) {
    session, err := store.Get(r, "name")
    if err != nil {
        http.Error(w, http.StatusText(500), 500)
        return // Forget this, and our handler won't explicitly return, causing unintended side effects
    }

   err := db.GetThing(thing)
        http.Error(w, http.StatusText(500), 500)
        return // Same again
    }
}

Since the compiler does not enforce naked returns on functions, your handler will continue, writing multiple response bodies and otherwise doing things you may not want. I typically only have 3-4 error returns in each handler, but across ~14 handlers (thus far) the potential for a typo starts to creep in.

My ServeHTTP method just has a switch case that handles specific error codes (e.g. 404, 500, 401, 403) and renders a specific "pretty" template for each one (or a default)—which saves me having to repeat myself in each handlers (I'm a fan of lean handlers as well).

I can also take a shortcut and write return 200, renderTemplate(...) — if renderTemplate returns an error (it writes to a *bytes.Buffer pool to ensure the template executes) then I can return a HTTP 500 instead without having mistakenly written to the header/response body or having to err := renderTemplate; if err != nil { ... } in every handler.

Still, this is why I like Go (it's flexible, but it's easy to understand) — hopefully the issue author finds this useful as well ;)

[matrixik]

http.Error(w, http.StatusText(500), 500)
return // Forget this, and our handler won't explicitly return, causing unintended side effects

Maybe that's a good candidate for new check in GoLint or go vet?

[azr]

Okay thank you guys for those infos, very interesting I will try everything here.

Still, if you do :

http.Error(w, http.StatusText(500), 500)
return

in a middleware handler, can it stop the execution and return an error ?

---

And in the code of the blog post they do :

c := appengine.NewContext(r) // I guess it's the full context of the request with some cool infos
c.Errorf("%v", e.Error)

Is there some simple way to get the full context of the request in goji too to log it ? (Like the IP of the user on the other side, the time, the url, the parameters, the path, the name of the matched route ( can I name a route ?), and more ?
In case of an error I would like to say some info to user ( but not too much ) in the format they asked (json or xml) and have more info to log, with logging levels etc.
May be that's my role to do the logging part but I'm also not sure where to do it, in a middleware handler may be ?

---

To finish anoying you ; I use gorm, and when nothing is found it returns an error, it's not such a big error but in this case I was doing something like this :

func VerifyErrors(query *gorm.DB) {
    if query.Error != gorm.RecordNotFound { // Pardon me this is my first application !!!!
        log.Panicf("gorm error : '%v'", query.Error)
    }
}

The more I look at it the worst it looks like.

[elithrar]

@Azer-

• As long as you don't call h.ServeHTTP(w, r) (which will invoke the next handler), the middleware chain will not continue. You can then call http.Error or http.Redirect or any other function you like.
• Goji has its own logger middleware.Logger (https://github.com/zenazn/goji/tree/master/web/middleware) that can log details about each request. I'm not familiar enough with App Engine to comment on its specifics, though.
• I wouldn't panic if the query has an error. Return an error back to your handler and then let your handler decide whether to 404 (if the record doesn't exist) or 500 (if the database can't be reached/gives a more permanent error).

[azr]

Awesome, thanks @elithrar :) I'll test all this !! I need more exp !

[saj1th]

@Azer- Perhaps the use of panic + recoverer-middleware might not be a good approach for control flow. Like @elithrar mentioned, it would be ideal to return an error back to the handler. Reserve a panic for the truly exceptional cases

I personally found @mmcgrana 's function to handle response interesting ~ https://github.com/mmcgrana/pgpin/blob/master/web.go#L39

It assumes JSON output and the same function is used for returning error and data - but worth taking a look

[azr]

Hey @saj1th, yes very interesting indeed, thanks :)
Interesting project too, I'll read their source !

[zenazn]

Since this issue hasn't seen any traffic in over a month I'm going to close it out. Feel free to open a new issue if you have any more questions—I'm always curious to see what kind of things people run into when using Goji!