Add support for http.MaxBytesHandler #355
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `ReadMaxBytes` option is nice, but we should also return properly-coded errors when `net/http.MaxBytesHandler` middleware is in play. This has no API surface area, but it's a nice improvement.
|
@robbertvanginkel I think this came up early in our discussion of the separation of concerns between |
twilly
reviewed
Sep 1, 2022
pkwarren
reviewed
Sep 2, 2022
|
|
||
| func asMaxBytesError(situation string, err error) *Error { | ||
| const expect = "http: request body too large" | ||
| text := err.Error() |
There was a problem hiding this comment.
Any benefit to using errors.Is here instead or will we never expect it to be wrapped?
There was a problem hiding this comment.
We can't use errors.Is on Go 1.18 - the http.MaxByteError type was introduced in Go 1.19. On 1.18, we should be okay because wrapping will still leave some extra context: http: request body too large as the error string, which we should detect down below.
I don't know what I was thinking here.
pkwarren
approved these changes
Sep 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
ReadMaxBytesoption is nice, but we should also returnproperly-coded errors when
net/http.MaxBytesHandlermiddleware is inplay. This has no API surface area, but it's a nice improvement.