proposal: net/http: export name for "request body too large" error returned by MaxBytesReader #41493
Comments
ianlancetaylor
changed the title
|
When do you need this? When does this need come up in practice? Thanks. CC @bradfitz |
|
Hello @ianlancetaylor, 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")
}
})
}
} |
|
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. |
|
@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. |
|
@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. |
|
@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) |
|
I use a
These are completely different errors in my opinion and I want to handle them as such. |
|
I currently stumbled on the same issue, and I do believe it is much friendlier to the user to return an exported error which allows the use of |
|
This is a dupe of #30715. |
|
Duplicate of #30715. Will comment on that one. |
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 errorhttp: request body too large.What did you expect to see?
https://play.golang.org/p/rm17KEL91Z5
What did you see instead?
https://play.golang.org/p/ucUuzXjY3s7
The text was updated successfully, but these errors were encountered: