refactor: refactoring http request creation and sending #395
Conversation
Codecov Report
@@ Coverage Diff @@
## master #395 +/- ##
==========================================
+ Coverage 95.26% 96.95% +1.69%
==========================================
Files 17 17
Lines 676 690 +14
==========================================
+ Hits 644 669 +25
+ Misses 22 15 -7
+ Partials 10 6 -4
|
|
@sashabaranov Please check it. |
vvatanabe
changed the title
|
This is super cool! 🚀 |
internal/request_builder.go
Outdated
| func (b *HTTPRequestBuilder) Build(ctx context.Context, | ||
| method, url string, body any, header http.Header) (req *http.Request, err error) { |
There was a problem hiding this comment.
nit: maybe let's go one-argument-per-line style here, it's a bit longer but seems cleaner. What do you think?
func (b *HTTPRequestBuilder) Build(
ctx context.Context,
method string,
url string,
body any,
header http.Header
) (req *http.Request, err error) {There was a problem hiding this comment.
| time.Sleep(10 * time.Nanosecond) | ||
| }) | ||
| ctx := context.Background() | ||
| ctx, cancel := context.WithTimeout(ctx, time.Nanosecond) |
There was a problem hiding this comment.
Wow, that's some bleeding edge timeout! 😄
There was a problem hiding this comment.
@sashabaranov I didn't want to spend time running tests.;D
Here is some background on this timeout test.
sendRequest, sendRequestRaw, and sendRequestStream each exec c.config.HTTPClient.Do(req) internally. This error handling was not covered test. That is why we implemented a workaround for c.config.HTTPClient.Do(req) with a timeout, to intentionally cause it to return an error.
It's hard to convey the intent of the test, isn't it? Would it be better to write a test that executes sendRequest, sendRequestRaw, and sendRequestStream directly?
| @@ -55,8 +92,6 @@ func (c *Client) sendRequest(req *http.Request, v any) error { | |||
| req.Header.Set("Content-Type", "application/json; charset=utf-8") | |||
| } | |||
|
|
|||
| c.setCommonHeaders(req) | |||
|
|
|||
| res, err := c.config.HTTPClient.Do(req) | |||
| if err != nil { | |||
| return err | |||
There was a problem hiding this comment.
I wanted to cover the above line.
#395 (comment)
http.Requestcreation and sending were duplicated, so they have been aggregated into a common function.RequestBuilder#Buildfunction calls were aggregated into theClient#newRequestfunction.http.Requestin theClient#newRequestfunction.http.Requestis now aggregated into the following three functions.Client#sendRequestfunction: marshalling the response into a structureClient#sendRequestRawfunction: returns the Body of the response as isClient#sendRequestStreamfunction: use generics to specify astreamabletype.