-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#457: allow to rewrite system error response #458
Conversation
I think this is a feature other people have requested before. I would like to see some changes to make this more usable before I can accept it. I think it would be useful to add some documentation on what kind of errors the user can expect as argument to the |
Thanks for review. All fixes are done |
7d8045c
to
aa16fe7
Compare
http.go
Outdated
@@ -1652,6 +1654,11 @@ func appendBodyFixedSize(r *bufio.Reader, dst []byte, n int) ([]byte, error) { | |||
} | |||
} | |||
|
|||
// ErrBrokenChunks is returned when server got broken chunked body (Transfer-Encoding: chunked). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to:
ErrBrokenChunks is returned when server receives a broken chunked body (Transfer-Encoding: chunked).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
http.go
Outdated
@@ -847,7 +847,9 @@ func (req *Request) Read(r *bufio.Reader) error { | |||
|
|||
const defaultMaxInMemoryFileSize = 16 * 1024 * 1024 | |||
|
|||
var errGetOnly = errors.New("non-GET request received") | |||
// ErrGetOnly is returned when server expects only GET requests, | |||
// but some other type of request came (Server.GetOnly option is enabled). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change enabled
to true
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
server.go
Outdated
@@ -152,6 +152,17 @@ type Server struct { | |||
// Handler for processing incoming requests. | |||
Handler RequestHandler | |||
|
|||
// ErrorHandler for returning an error response in user-defined way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to:
ErrorHandler for returning a response in case of an error while receiving or parsing the request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
server.go
Outdated
@@ -152,6 +152,17 @@ type Server struct { | |||
// Handler for processing incoming requests. | |||
Handler RequestHandler | |||
|
|||
// ErrorHandler for returning an error response in user-defined way. | |||
// | |||
// Following errors will be captured with this handler: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I'm not sure if these are really all the errors (I think more io.*
errors are possible it's probably better to change this to:
The following is a non-exhaustive list of errors that can be expected as argument:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
http.go
Outdated
@@ -1652,6 +1654,11 @@ func appendBodyFixedSize(r *bufio.Reader, dst []byte, n int) ([]byte, error) { | |||
} | |||
} | |||
|
|||
// ErrBrokenChunks is returned when server got broken chunked body (Transfer-Encoding: chunked). | |||
type ErrBrokenChunks struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being pedantic but can you change this to ErrBrokenChunk
. The error is only about one chunk being broken, not multiple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Sorry about my english %) Just I need more practice. |
No description provided.