結局 golang の HTTP Response Body はどう閉じるのが正しいのか?

さっそくカウンター記事を頂きました.ありがとうございます!

qiita.com

たしかに必ず閉じてるように見えます.エラーがあっても Body が non-nil なら閉じるようにコード仕込んでおくというのは杞憂だったのでしょうか.そんな気もします.たぶん誰もそんな風にコード書いてないし,godoc のサンプルにもそういうのはない.(僕も元記事読んだときは衝撃的だった

エラーがないときは Response Body が non-nil であることが godoc に書かれています. http://golang.org/pkg/net/http/#Client.Do

なので,エラーがないときは Response Body を閉じるようにコードを仕込んでおく必要があります. それは gopher の共通認識な感じがしてるんですが,問題なのはエラーがあるときです.

エラーがあるときは,たいていの場合 Response が nil です.なので,こういうコードを書いていると panic します.

package main

import (
    "fmt"
    "net/http"
)

func main() {
    resp, err := http.Get("piyopiyo")
    defer resp.Body.Close()  // ← Response が nil のとき呼ばれると panic するのでこの位置はダメ
    if err != nil {
        fmt.Println(err)
    }
}

で,やっかいなのは,エラーがあるのに Response が non-nil の場合があることです. これはリダイレクトの処理の時に起こります.リダイレクトでエラーがあると,最後にリダイレクトしたアドレスを返すために Response は non-nil です.resp.Request.URL.String() を見ると最後のURLが分かります.で,このとき,Body には nil が入ってるならそれはそれで安心なんですがどうも non-nil なようです.ただし,この Response Body はコード上必ず Close されてるようなので,わざわざ Close するコードを用意することはないのではないか?というのが書いていただいた記事の意図するところだと思います.

なので,リダイレクトのエラーの時,Response Body が Close されてるのは仕様なの?というのがはっきりすれば,エラー処理の書き方もスッキリするわけなんですが,こんな Issue がありました.

github.com

うーん,仕様なんですかね,どうなんですかね.

詳しい方のご意見を引き続きお待ちしております mm

追記

go のことは go に聞け

たしかに!

早速コード拾ってきて grep かけてみよう

$ git clone git@github.com:golang/go.git
$ cd go; git grep -A 3 -B 10 -e "Body.Close"

お,このパターン,1個だけあった.

// Issue 6981
func TestTransportClosesBodyOnError(t *testing.T) {
        if runtime.GOOS == "plan9" {
                t.Skip("skipping test; see http://golang.org/issue/7782")
        }
        defer afterTest(t)
        readBody := make(chan error, 1)
        ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
                _, err := ioutil.ReadAll(r.Body)
                readBody <- err
        }))
        defer ts.Close()
        fakeErr := errors.New("fake error")
        didClose := make(chan bool, 1)
        req, _ := NewRequest("POST", ts.URL, struct {
                io.Reader
                io.Closer
        }{
                io.MultiReader(io.LimitReader(neverEnding('x'), 1<<20), errorReader{fakeErr}),
                closerFunc(func() error {
                        select {
                        case didClose <- true:
                        default:
                        }
                        return nil
                }),
        })
        res, err := DefaultClient.Do(req)
        if res != nil {
                defer res.Body.Close()
        }
        if err == nil || !strings.Contains(err.Error(), fakeErr.Error()) {
                t.Fatalf("Do error = %v; want something containing %q", err, fakeErr.Error())
        }
        select {
        case err := <-readBody:
                if err == nil {
                        t.Errorf("Unexpected success reading request body from handler; want 'unexpected EOF readi
ng trailer'")
                }
        case <-time.After(5 * time.Second):
                t.Error("timeout waiting for server handler to complete")
        }
        select {
        case <-didClose:
        default:
                t.Errorf("didn't see Body.Close")
        }
}

この Issue が関係ありそう.調べてみよう.

github.com

エラーの時は Response Body を閉じるように修正が入ったように見える. 1.3からはエラーじゃないときだけ Body を閉じれば大丈夫なんじゃないかな. うえのテストのコードで,エラーの時も Body を閉じるコードが書いてあるのは,テストで失敗したときでも Body を閉じるためじゃないかな.

で,リダイレクトのエラーの時もこの方針が守られてるかが問題だな.


追記:解決編 → 「たぶんみんな間違えてる golang の HTTP Respose Body の閉じ方」は間違えてる - 押してダメならふて寝しろ