Go: doc/articles/wiki: "Writing Web Applications" example writes the reponse header twice

[Houndie]

What version of Go are you using (go version)?

go1.11

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN="/home/jabenze/go/bin"
GOCACHE="/home/jabenze/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jabenze/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build876174949=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Follow the "Writing Web Applications" tutorial here, up through the error handling section here: https://golang.org/doc/articles/wiki/#tmp_9. L

Look at the function renderTemplate, reproduced here:

func renderTemplate(w http.ResponseWriter, tmpl string, p *Page) {
    t, err := template.ParseFiles(tmpl + ".html")
    if err != nil {
        http.Error(w, err.Error(), http.StatusInternalServerError)
        return
    }
    err = t.Execute(w, p)
    if err != nil {
        http.Error(w, err.Error(), http.StatusInternalServerError)
    }
}

Modify the Page data structure by removing a field (or modify the template to reference a nonexisting field) so that the line: err = t.Execute(w, p) will cause an error to be created. Observe the response code sent by the application. The following line: http.Error(w, err.Error(), http.StatusInternalServerError) would imply the response code to be 500, however, the application returns 200.

This is because t.Execute is calling w.Write which, because w.WriteHeader has not yet been called, causes an implicit call to w.WriteHeader(200). Further calls to w.WriteHeader, such as the one inside http.Error are ignored.

There is no way to able to send a 500 error code in the case of all errors, since w.Write itself can return an error, and at some point, one has to simply log an error and return without setting an error code. Optionally, the example could render to a bytes.Buffer so that template rendering errors could be caught.

What did you expect to see?

Based on the code, a 500 error code

What did you see instead?

Error code 200.

[dmitshur]

This is a valid issue, good catch! Thank you for the well-written report and good analysis.

We should discuss how to resolve it. As you said, one option is to render the template to a bytes.Buffer first, and only write to the http.ResponseWriter once all the operations that might fail have succeeded, leaving only copying the buf into rw. However, this might not fit well with the rest of the tutorial.

There are other ways to fix this too, and it's important to come up with a solution that makes the most sense in the context of the "Writing Web Applications" tutorial and what the reader is expected to know or be exposed to by then.

/cc @adg as the original author of the article, I believe.

[Houndie]

The only problem with the bytes buffer rendering is that, if the tutorial is going to cover proper error handling, we still run into the same problem down the line.

buf := &bytes.Buffer{}
if err := t.Execute(buf, p); err != nil {
   http.Error(w, err.Error(), http.StatusInternalServerError)
   return
}

if err := w.Write(buf.Bytes()); err != nil {
   // can't call http.Error here for the same reason as before.  We've caught 
   // template problems, but we still can't do exhaustive error handling with http.Error
}

My suggestion for the tutorial would probably just be to keep writing directly to the ResponseWriter and log the template rendering error, and then have either a comment or short blurb in the tutorial about why we can't write the header twice. It's correct code and doesn't complicate the tutorial too much.

Dunno if there's a simpler solution that I'm missing though.

[adg]

My suggestion for the tutorial would probably just be to keep writing directly to the ResponseWriter and log the template rendering error

This is what I came here to say. Template rendering errors are for the application author anyway, not the user, and should typically only happen during development of the program. Logging it here would be ideal, and a couple of sentences explaining this would actually improve the tutorial.

[gopherbot]

Change https://golang.org/cl/136757 mentions this issue: doc/articles/wiki: remove misleading code in tutorial

[Houndie]

I'm aware this still has the "Needs Decision" tag associated with it, but since all were in agreement so far, I went ahead and wrote something up to fix it. I'm not the best writer, but it's a decent start at least.