CODEWITHSUNDEEP
go
Sunder
Sunder

Reputation: 1513

Safely terminating a request outside of handlefunc

How can I safely terminate a request outside of a dispatcher (handlefunc) without an explicit return in handlefunc? In the code below, I end up seeing both "Bad request" and "Not found" in my response, but I'd like f() to abort the request -- so I wouldn't have to clutter dispatcher with error checks and returns.

package main

import (
    "net/http"
)

func f(w http.ResponseWriter, r *http.Request, k string) string{
    if v, ok := r.Form[k]; !ok{     
        http.Error(w, "Bad request", http.StatusBadRequest)
        return ""
    }else{
        return v[0]
    }
}

func main(){
    http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request){
        foo := f(w, r, "foo") //make sure foo is passed, if not abort with "bad request" without requiring an explicit return here
        bar := f(w, r, "bar") //make sure bar is passed, if not abort with "bad request" without requiring an explicit return here
        //lookup in db...
        //abort with a 404 if not found in db
        http.NotFound(w, r)         
    })  

    http.ListenAndServe(":6000", nil)
}

The above was an attempt to refactor the following:

func main(){
    http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request){
        if foo,ok:=r.Form["foo"];!ok{
            http.Error(w, "Bad request", http.StatusBadRequest)
            return
        }
        if bar,ok:=r.Form["bar"];!ok{
            http.Error(w, "Bad request", http.StatusBadRequest)
            return
        }
        //lookup in db...
        //abort with a 404 if not found in db
        http.NotFound(w, r)         
    })  

    http.ListenAndServe(":6000", nil)
}

Upvotes: 0

Views: 124

Answers (1)

twotwotwo
twotwotwo

Reputation: 30087

I don't think trying to abort from within a function is what you want to do here. Instead, make the validation less repetitive. You could write something like package validate with func Require(form url.Values, required ...string) error, and, in your handler, if err := validate.Require(r.Form, "foo", "bar"); err != nil { ... }.

Or you could do a more general validator: maybe take a map of field name to validation type (number, string, etc.) and return another map that's nil if no errors and {"fieldName": error} otherwise.

Redditors talked some about this and one wrote a library that uses struct tags to validate. There's another struct-based validation implementation in a new form rendering/validation toolkit. (I've tried neither.) The Redditors raised the tricky question of how much you can abstract validation before getting a "framework" that gets in the way when your app gets more complicated, and I don't have a simple answer.

There are cases where something really unexpected happens and the server can't do anything better than give the user an opaque "500 Internal Server Error" response, but the problem manifests deep below your HandlerFunc. The canonical example is a programming error like a nil pointer dereference. The advice from the Go team is to use panic when "the error is unrecoverable." (A tool like negroni.Recovery can help log panics, etc.)

Unnecessary panics are lame because:

  • panics make it easier to forget necessary error handling entirely, because it's implied that things can panic but explicit they can err
  • panic/recover error handling code is ugly and hard to maintain
  • panics introduce inconsistency with the standard err returns
  • panics use stack unwinding, which is much slower than normal control flow

I did a quick grep and essentially all calls to panic in the standard library and other official repositories are there to catch programming/internal errors, not crash on surprise external conditions like bad input or I/O errors. Things like file/object not found or invalid user input can usually be handled more gracefully than by crashing the request (it's at least possible to return a specific error), and they aren't really unexpected (it's the Internet; you'll get invalid input and requests for things that don't exist). So panic is not for normal request aborts--it's for crashing when what "can't happen" has happened and the request can't continue.

Again, here, I'd handle invalid input with a standard if block in the handler function, but rearrange your validation so that it doesn't require a lot of code per required argument.

Upvotes: 2

Related Questions