fix: ensure bufferWriter returns an error when exceeding maxResponseBodyBytes #249
Conversation
| outReq := b.copyRequest(req, body, totalSize) | ||
|
|
||
| defer func() { | ||
| if err := recover(); err != nil { |
There was a problem hiding this comment.
with this recover, you will catch all the panic from other middleware.
| return | ||
| } | ||
|
|
||
| panic(err) |
There was a problem hiding this comment.
Not sure about that.
Why do you not write the error inside the response?
There was a problem hiding this comment.
The fact that the Write method of bufferWriter doesn't return an error prevents ErrAbortHandler from being raised.
The idea is to fix the Write method and specifically handles ErrAbortHandler by returning a 500 status code to avoid breaking changes.
For other errors, a panic is raised to ensure that existing panic behaviors remain unchanged.
After giving it some thought, I wonder if there might be impacts I didn't anticipate. Perhaps an approach like this (#247) might be safer.
|
🤔 the implementation inside #247 seems a better approach, can you provide more explanation? |
|
I agree with you, #247 seems like a safer approach. I close this PR accordingly. |
2 participants
Description
This PR ensures that
bufferWriterreturns an error when the underlying writer fails.It fixes cases where the buffer incorrectly returns a 200 status code even when
maxResponseBodyBytesis exceeded (see the unit test update for details).related to #247