check response size for debug logs #99
Conversation
Codecov Report
@@ Coverage Diff @@
## master #99 +/- ##
==========================================
- Coverage 97.01% 96.93% -0.09%
==========================================
Files 10 10
Lines 973 979 +6
==========================================
+ Hits 944 949 +5
- Misses 15 16 +1
Partials 14 14
Continue to review full report at Codecov.
|
If response size exceeds 1000000, do not log response.
sudo-suhas
force-pushed
the
debug-log-res-size
branch
from
de031c4 to
afc9b30
Compare
response.go
Outdated
| if r.body != nil { | ||
| if len(r.body) > 1000000 { |
There was a problem hiding this comment.
What is this 1000000 value? are you picking random max value to limit it? and why?
There was a problem hiding this comment.
Yea.. It is arbitrary. What do you suggest? Should I add a var so that the user can override if required?
There was a problem hiding this comment.
Making this value configurable at client level would be best. Since some user wants to get complete body value.
- Add an un-exported field
debugBodySizeLimit int64toClientand method in the clientSetDebugBodyLimit(sl int64) - Initializing
debugBodySizeLimitdefault value asmath.MaxInt32(this is good enough to begin with) at client instance creation - Add corresponding method for default resty client in default.go
This way it will satisfy possibilities.
response.go
Outdated
| ct := r.Header().Get(hdrContentTypeKey) | ||
| if IsJSONType(ct) { | ||
| out := acquireBuffer() | ||
| defer releaseBuffer(out) | ||
| if err := json.Indent(out, r.body, "", " "); err == nil { | ||
| bodyStr = string(out.Bytes()) | ||
| err := json.Indent(out, r.body, "", " ") |
There was a problem hiding this comment.
This could be better and concise.
if err := json.Indent(out, r.body, "", " "); err == nil {
return out.String()
}There was a problem hiding this comment.
That results in a warning from golint - if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (golint).
There was a problem hiding this comment.
Actually remove this line return "unexpected error: " + err.Error() with my above code snippet then golint warning goaway. Because if any errors in formatting debug log, return the original body value which is r.String().
|
Also test coverage is decreasing, spend some time on it. Thank you. |
|
Could you tell me where I would need to add tests? I did not see |
|
To improve test you need create a scenario for response body limit. With configurable value you can decrease the value to create a scenario. You can add the test in Use following commands to know coverage lines info: |
- client.go
- Add private field `Client.debugBodySizeLimit` for setting response size
limit used for debug logs.
- Add method `Client.SetDebugBodyLimit` for setting
`Client.debugBodySizeLimit`.
- default.go
- Set `debugBodySizeLimit` to `math.MaxInt32` while creating new resty
client.
- Add mothod `SetDebugBodyLimit` for setting the limit for `DefaultClient`.
- response.go>fmtBodyString
- Add method parameter for debug response size limit
- Include response body size in debug logs when it exceeds limit.
- Fallback to `r.String()` if `json.Indent` errors.
- middleware.go - Pass `debugBodySizeLimit` to `fmtBodyString`.
- client_test.go
- Add test for `SetDebugBodyLimit`.
- Add tests for debug body size limit.
- resty_test.go>createGetServer
- Refactor `if-else-if-else` chain to use switch(recommended by Effective Go).
- Add cases `/json`, `/json-invalid`, `/long-text`, `/long-json`
|
@sudo-suhas Thanks for your contribution and improving tests. |
In my case, for some endpoints, the response was too large(ex: length - 1616599). This made it very difficult to use debug mode, which btw, is really awesome. This PR checks the size of the response and if it exceeds 1000000,
***** RESPONSE TOO LARGE *****is logged instead.