読者です 読者をやめる 読者になる 読者になる

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

go golang

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

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 を閉じるためじゃないかな．

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