Go: proposal: net/http: export name for "request body too large" error returned by MaxBytesReader

[jerome-laforge]

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

Go 1.15.2

Does this issue reproduce with the latest release?

yes

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

All

What did you do?

With http.MaxByteReader, I want easily check (for example with switch case statement) the error http: request body too large.

What did you expect to see?

https://play.golang.org/p/rm17KEL91Z5

func TestMaxBytesReader(t *testing.T) {
    req := ioutil.NopCloser(bytes.NewBufferString("my too long request"))
    _, err := http.MaxBytesReader(httptest.NewRecorder(), req, 2).Read([]byte{10: 0})
    if err != http.ErrBodyTooLarge {
        t.FailNow()
    }
}

What did you see instead?

https://play.golang.org/p/ucUuzXjY3s7

func TestMaxBytesReader(t *testing.T) {
    req := ioutil.NopCloser(bytes.NewBufferString("my too long request"))
    _, err := http.MaxBytesReader(httptest.NewRecorder(), req, 2).Read([]byte{10: 0})
    if err == nil || err.Error() != "http: request body too large" {
        t.FailNow()
    }
}

[ianlancetaylor]

When do you need this? When does this need come up in practice? Thanks.

CC @bradfitz

[jerome-laforge]

Hello @ianlancetaylor,
In practice, in our REST API, we want easily check error with switch case statement and reply by an error accordingly to our specification.

for example:

func TestDecode(t *testing.T) {
    type errorBody struct {
        Code    int
        Message string
    }

    testCases := []struct {
        name               string
        body               string
        expectedStatusCode int
        expectedCode       int
    }{
        {
            name:               "too long request",
            body:               `{"Message":"my too long request"}`,
            expectedStatusCode: http.StatusRequestEntityTooLarge,
            expectedCode:       12,
        },
        {
            name:               "unexpected EOF",
            body:               `{"M`,
            expectedStatusCode: http.StatusBadRequest,
            expectedCode:       34,
        },
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            w := httptest.NewRecorder()
            reqBody := struct{ Message string }{}
            req := ioutil.NopCloser(bytes.NewBufferString(tc.body))
            r := http.MaxBytesReader(w, req, 5)

            err := json.NewDecoder(r).Decode(&reqBody)
            if err != nil {
                var (
                    errResBody errorBody
                    statusCode int
                )

                switch err {
                case http.ErrBodyTooLarge:
                    statusCode = http.StatusRequestEntityTooLarge
                    errResBody = errorBody{
                        Code:    12,
                        Message: "blah blah",
                    }

                case io.ErrUnexpectedEOF:
                    statusCode = http.StatusBadRequest
                    errResBody = errorBody{
                        Code:    34,
                        Message: "blah blah",
                    }

                default:
                    // unspecified error case
                    statusCode = http.StatusBadRequest
                    errResBody = errorBody{
                        Code:    56,
                        Message: "blah blah",
                    }
                }

                w.WriteHeader(statusCode)
                json.NewEncoder(w).Encode(errResBody)
            }

            resp := w.Result()
            actualBody := errorBody{}
            json.NewDecoder(resp.Body).Decode(&actualBody)

            if resp.StatusCode != tc.expectedStatusCode {
                t.Error("unexpected status code")
            }

            if actualBody.Code != tc.expectedCode {
                t.Error("unexpected code")
            }
        })
    }
}

[ianlancetaylor]

I'm not clear: is this just for testing purposes?

For testing purposes I tend to think it's usually best to check for the presence of an error, rather than for an exact error. Users won't usually care about an exact error.

I think that the best way to decide whether we should export this error is whether ordinary non-test code needs to distinguish this case.

[psampaz]

@ianlancetaylor if you want to check for this error in non test code, in order to reply explicitly with http.StatusRequestEntityTooLarge, what would be the best approach to do it?

This is requirement from a real project and I suppose not something very uncommon.

[ianlancetaylor]

@psampaz Can you show us the kind of code you want to write? This proposal would be stronger with an example of where the information is needed by non-test code. Thanks.

[psampaz]

@ianlancetaylor something similar to this:

            body = http.MaxBytesReader(w, r.Body, bodySizeLimit)

            if _, err := ioutil.ReadAll(body); err != nil {
                if err.Error() == "http: request body too large" {
                    http.Error(w, err.Error(), http.StatusRequestEntityTooLarge)
                    return
                }

                http.Error(w, err.Error(), http.StatusBadRequest)
                return
            }

The following

if err.Error() == "http: request body too large" {

could be turned to:

errors.Is(err, http.ErrBodyTooLarge)