An enhanced HTTP client for Go
Home Page: http://gojek.tech
License: Apache License 2.0
Middleman-Powered GO-JEK Technologies's Website 👻 🕸 hosted with ❤️ by GitHub
GO-JEK Technologies generates matter for GO-JEK Technologies on GitHub Pages
try,
make clean
make setup
make install
make serve
or just,
make
Copyright 2018, GO-JEK Technologies <http://www.gojek.io>
adding following test into hystrix_client_test.gowill cause data race using go test -race
func TestHystrixHTTPClientGetReturnedURLTimeout(t *testing.T) {
client := NewClient(
WithHTTPTimeout(5*time.Millisecond),
WithCommandName("some_command_name"),
WithHystrixTimeout(20*time.Millisecond),
WithMaxConcurrentRequests(2000),
WithErrorPercentThreshold(10),
WithSleepWindow(100),
WithRequestVolumeThreshold(10000),
)
dummyHandler := func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, http.MethodGet, r.Method)
assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
assert.Equal(t, "en", r.Header.Get("Accept-Language"))
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{ "response": "ok" }`))
time.Sleep(10 * time.Millisecond)
}
server := httptest.NewServer(http.HandlerFunc(dummyHandler))
defer server.Close()
headers := http.Header{}
headers.Set("Content-Type", "application/json")
headers.Set("Accept-Language", "en")
wg := sync.WaitGroup{}
for n := 0; n < 100; n++ {
wg.Add(1)
go func() {
defer wg.Done()
_, err := client.Get(server.URL, headers)
require.Error(t, err, "should having Client.Timeout exceeded while awaiting headers error")
assert.IsType(t, &url.Error{}, err)
}()
}
wg.Wait()
}
Log:
WARNING: DATA RACE
Write at 0x00c00020a5c0 by goroutine 99:
github.com/gojektech/heimdall/hystrix.(*Client).Do()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client.go:181 +0x3e1
github.com/gojektech/heimdall/hystrix.(*Client).Get()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client.go:105 +0xca
github.com/gojektech/heimdall/hystrix.TestHystrixHTTPClientGetReturnedURLTimeout.func2()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client_test.go:566 +0xab
Previous write at 0x00c00020a5c0 by goroutine 138:
github.com/gojektech/heimdall/hystrix.(*Client).Do.func1()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client.go:182 +0xe4
github.com/afex/hystrix-go/hystrix.Do.func1()
/Users/ardhi/gopath/src/github.com/afex/hystrix-go/hystrix/hystrix.go:179 +0x4a
github.com/afex/hystrix-go/hystrix.Go.func3()
/Users/ardhi/gopath/src/github.com/afex/hystrix-go/hystrix/hystrix.go:140 +0x1d3
Goroutine 99 (running) created at:
github.com/gojektech/heimdall/hystrix.TestHystrixHTTPClientGetReturnedURLTimeout()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client_test.go:564 +0x370
testing.tRunner()
/usr/local/Cellar/go/1.11.5/libexec/src/testing/testing.go:827 +0x162
Goroutine 138 (running) created at:
github.com/afex/hystrix-go/hystrix.Go()
/Users/ardhi/gopath/src/github.com/afex/hystrix-go/hystrix/hystrix.go:96 +0x551
github.com/afex/hystrix-go/hystrix.Do()
/Users/ardhi/gopath/src/github.com/afex/hystrix-go/hystrix/hystrix.go:200 +0x32d
github.com/gojektech/heimdall/hystrix.(*Client).Do()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client.go:181 +0x3b9
github.com/gojektech/heimdall/hystrix.(*Client).Get()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client.go:105 +0xca
github.com/gojektech/heimdall/hystrix.TestHystrixHTTPClientGetReturnedURLTimeout.func2()
/Users/ardhi/gopath/src/github.com/gojektech/heimdall/hystrix/hystrix_client_test.go:566 +0xab
The intention may have been to use *.WithHTTPTimeout
whould be awesome if heimdall also capture response time and post to statsd
heimdall should allow pluggable cache based on etags.
When attempting to use constant backoff strategy, I found that on the first retry of a request, the actual backoff time is 0. That's because on the first retry the client calls c.retrier.NexInterval(0) so constantBackoff.Next(0) returns 0:
Line 34 in 16e67fd
I am using go1.14.
I tried to import v5.1.0 with this command:
go get -u github.com/gojek/[email protected]
and it gave me this error:
go get github.com/gojek/[email protected]: github.com/gojek/[email protected]: invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v5
As stated in go mod wiki page, any package with major version larger than v0 and v1 must include its major version in the module name.
So the statement here module github.com/gojektech/heimdall should be appended with /v5 so that it would become module github.com/gojektech/heimdall/v5.
I don't know whether this change would broke backward compatibility for imports.
Example: will import "github.com/gojektech/heimdall/httpclient" be changed to import "github.com/gojektech/heimdall/v5/httpclient" and all will be ok?
If all is OK I can make a PR to this.
I want an SSE connection to be established for reading messages from the server, with predefined controls for circuit breaking. Do you have something that can work?
Hi guys, I don't see any example of concurrent usage. Is it safe to use the same the same client object from multiple go-routine?
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(500)
}))
defer server.Close()
retryCalled := 0
httpclient := httpclient.NewClient(
httpclient.WithRetrier(heimdall.NewRetrierFunc(func(_ int) time.Duration {
retryCalled++
return 1 * time.Second
})),
httpclient.WithRetryCount(0),
)
req, err := http.NewRequest(http.MethodGet, server.URL, nil)
assert.NoError(t, err)
res, err := httpclient.Do(req)
assert.NoError(t, err)
_ = res
assert.Equal(t, 0, retryCalled)retrier still called with 0 retry count, is this expected?
What is the reason to have two timeouts: HystrixTimeout and HTTPTimeout for hystrix.Client ? It seems that two points are valid for current implementation:
timeout field in hystrix.Client is set but never used (maybe bug ?)HTTPTimeout < HystrixTimeout, then HystrixTimeout is never used. And if HystrixTimeout < HTTPTimeout then hystrix will return error but goroutine with httpclient.Do will be still in blocking state waiting for response or http timeout (problem in hystrix code).https://github.com/gojektech/heimdall/blob/badd722da88e975e3f787abdd0c0c068e7b74b80/hystrix_client.go#L54
I think you mean
// SetCustomHTTPClient sets custom HTTP client
We have a common big interface for retrier related methods vs those for making http requests.
We could have Client interface for HTTP related methods and Retriable interface for retrier related methods, segregating the interfaces
https://godoc.org/golang.org/x/net/context/ctxhttpfunc NewHTTPClient(timeoutInMilliseconds int) Client in http_client accepts int but could be time.Duration, so its flexible
it could be a heimdall.Config{...} so we could add more later.
my http get method call using heimdall
timeout := 1000 * time.Second
client := httpclient.NewClient(httpclient.WithHTTPTimeout(timeout)
req, err := client.Get(api, r.Header)--
If i set the header its working else its an unmarshal error
heimdall code
// Get makes a HTTP GET request to provided URL
func (c *Client) Get(url string, headers http.Header) (*http.Response, error) {
var response *http.Response
request, _ := http.NewRequest(http.MethodGet, url, nil)
request.Header.Set("Authorization", headers["Authorization"][0])
request.Header = headers
//if i add the below line code my api returns success response
request.Header.Set("Authorization", request.Header["Authorization"][0])
return c.Do(request)
}Hi there!
I think there is a problem with MaxConcurrentRequests: It is defined here, but never used. How can I limit concurrent requests then?
Hello,
Thank you a lot for this amazing library, it's really useful and well thought.
How would you feel about supporting function options? Something like this:
hystrixClient := heimdall.NewHystrixHTTPClient(
timeout,
heimdall.WithCommandName("MyCommand"),
heimdall.WithHystrixTimeout(1100),
heimdall.WithMaxConcurrentRequests(100),
heimdall.WithErrorPercentThreshold(25),
heimdall.WithSleepWindow(10),
heimdall.WithRequestVolumeThreshold(10),
heimdall.WithHystrixRetryCount(2),
heimdall.WithHystrixRetrier(heimdall.NewRetrier(heimdall.NewConstantBackoff(10, 5))),
)Is this something you'd like to see in the lib? If yes I can send a PR with it.
Thanks again!
apparently it's because the request body is already read and not restored before retrying
here's a possible solution https://medium.com/@xoen/golang-read-from-an-io-readwriter-without-loosing-its-content-2c6911805361
and it's likely happening here:
https://github.com/gojektech/heimdall/blob/master/hystrix/hystrix_client.go#L152
When comparing https://github.com/gojek/heimdall/blob/master/httpclient/client.go and https://github.com/gojek/heimdall/blob/master/hystrix/hystrix_client.go it seems that they only differ fundamentally in their respective Do methods. One could factor that part out into seperate Doer implementations and only have one httpclient implementation that delegates to a configurable Doer.
I would be willing to file a PR if that is acceptable.
Need to update the README to say:
import "github.com/gojektech/heimdall/httpclient"
instead of "github.com/gojektech/heimdall".
Hello 👋
I have a question regarding the Hystrix implementation.
As most of you probably already know Hystrix is no longer supported by Netflix and they are moving towards a more adaptive concurrency limits. They are only improving in the resilience4j.
I do understand that Hystrix is more a pattern used for resilience than a lib, but if there is no further development in improving it maybe we should rethink if it makes sense to keep using it?
Is there any plans for this lib to shift to the same idea?
Let me know your thoughts.
Thanks!
Need to change the first plus sign here to a multiplication:
Line 63 in 16e67fd
Exponential backoff with exponent factor 2 should be initialBackoff * 2^n.
In one of the examples in the README.md the client gets configured with hystrix.WithHystrixTimeout(1100). This is misleading because it would be interpreted as 1100 nanoseconds and not milliseconds. And since heimdall converts this duration at some point to milliseconds, the duration drops to 0 because of rounding. Which causes a default duration of 1000ms to take effect.
So everybody who uses this example gets a timeout of 1000ms and not 1100ms.
I suggest changing this to hystrix.WithHystrixTimeout(1100 * time.Millisecond).
Currently the timeout being set is by type casting time.Duration to an int; this is not how you should be converting time.
hystrix.ConfigureCommand(client.hystrixCommandName, hystrix.CommandConfig{
Timeout: int(client.hystrixTimeout),
MaxConcurrentRequests: client.maxConcurrentRequests,
RequestVolumeThreshold: client.requestVolumeThreshold,
SleepWindow: client.sleepWindow,
ErrorPercentThreshold: client.errorPercentThreshold,
})https://github.com/gojektech/heimdall/blob/master/hystrix/hystrix_client.go#L70
github.com/afex/hystrix-go/hystrix expects the timeout to be in milliseconds but an int on time.Duration would convert this to nanoseconds and not milliseconds.
P.S: Also interestingly with default values this conversion to panic with integer overflow.
See comment in #34
Thundering herd problem
Minor typo In Example (Creating a hystrix-like circuit breaker with fallbacks): hystrixWithRequestVolumeThreshold should be hystrix.WithRequestVolumeThreshold
ability to send custom logger on http request logger plugin would be helpful
Current Working Code which having Raw http and heimdallHttpClient (internal Api)
the below gist code having both success case and error case
https://gist.github.com/MuthukumarHelios/79ef131001a9f8fb5b2b4232c63c6b50
What if I need access to the response headers? Right now the Response [1] struct has no way to get them.
[1] https://github.com/gojektech/heimdall/blob/master/response.go#L4
Do(* http.Requestt) (http.Response, error)
Sticking to http contract would help us to use this library in existing codebase, also using the *http.Response wouldn't hide information the default functionality.
Problem:
When I use hystix client, I want an ability to use a framework like https://github.com/h2non/gock to mock traffic for my testing purposes.
gock provides a means of doing roughly
hystrix := NewHystrixClient()
gock.InterceptClient(hystrix)
gock.New("http://foo.com").
Get("/bar").
Reply(http.StatusOK).
JSON(map[string]string{"foo": "bar"})
Currently the do function is private
https://github.com/gojektech/heimdall/blob/6ce64ecf951195bdff9db6b754d81253a22f4235/hystrix_client.go#L124
This prevents from using heimdall as true http.Client wrapper.
https://godoc.org/golang.org/x/net/context/ctxhttpThis is the result from using normal http client vs heimdall client
( direct http client )
Time: 613.24249ms
Time: 73.776966ms
Time: 56.926805ms
Time: 76.188755ms
Time: 52.602725ms
Time: 181.545667ms
Time: 68.099089ms
Time: 80.868807ms
Time: 57.184804ms
Time: 61.28775ms
( heimdall)
Time: 440.696523ms
Time: 217.184449ms
Time: 263.74181ms
Time: 216.604801ms
Time: 380.274223ms
Time: 193.812942ms
Time: 227.070945ms
Time: 248.829814ms
Time: 239.477054ms
Time: 273.660091ms
Currently looks like the retry mechanism will only engage if there's an actual error in sending the request, or if it's a 5xx response code.
It would be handy to be able to configure it so that it will retry on a non-2xx response code. I can do a PR if the maintainers are interested.
(1) is pretty straightforward to fix (close the body before the next retry)
For (2) we need to decide whether to send an error for 5xx http status codes. If we decide to omit the error, this would lead to a change in contract, and we would potentially have to bump the major version
Usecase:
After using the HTTP client to Do the request. We want to directly unmarshal the response body into a certain struct. If it fails, we just want to know the status code of the API and returns accordingly.
The snippet of working code will be like:
responseBody := YourProgramResponseBodyStruct{}
httpStatusCode, err := heimdall.ResponseBody(req, &responseBody)
What good it will do:
It reduces the need to write again and again for decoding the response body. Saving lines of code with this abstraction
if request.Body != nil {
reqData, err := ioutil.ReadAll(request.Body)
if err != nil {
return nil, err
}
bodyReader = bytes.NewReader(reqData)
request.Body = ioutil.NopCloser(bodyReader) // prevents closing the body between retries
}
The above piece of code form client.go is using ioutil.ReadAll to read all data from reqeust body. Doesn't that process have performance issue while uploading large files?
There are several cases where you may want to use a custom http client, for example if I want to sign every request, or setting a custom cookie jar, adding custom logs to every request, sharing http client between libraries, etc...
Especially explaining the configuration options
Hi.
Does it really make sense, that the http retry mechanism is outside the CB call?
For me this seems very unintuitive:
When the CB is open, there will still be retries and also time.Sleep() backoffs. Isn't the intent of an opened CB also for the calling system to get a fast feedback and don't waste time?
Even on a completely new request there is no check if the cb is currently open. Why would one do retries and spend time waiting on an open CB?
For me it would make more sense to do the retries inside the cb. So you define the following behavior:
Or maybe I do get the idea wrong.
Please help clarify.
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
Django
The Web framework for perfectionists with deadlines.
Laravel
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
Microsoft
Open source projects and samples from Microsoft.
Google
Google ❤️ Open Source for everyone.
Alibaba
Alibaba Open Source for everyone
D3
Data-Driven Documents codes.
Tencent
China tencent open source team.