Request: Fix http.Client race issue when setting transport layer prox…

…y and timeouts (thrasher-corp#885) * backtester/request: trying to fix panic (WIP) * request: fix race for transport layer * request: linter issue fix * request: more linter issues * requester: Add function to remove the tracking of underlying http client and add to engine unload exchange. * request: add more context to error return * request: Fix after cherry pick issues * request: fix niterinos * exchanges: change return to package variable * request: changed named Co-authored-by: Ryan O'Hara-Reid <[email protected]>
AbdulwahabNour · Feb 17, 2022 · 6127e2a · 6127e2a
1 parent 11da520
commit 6127e2a
Show file tree

Hide file tree

Showing 53 changed files with 751 additions and 256 deletions.
diff --git a/cmd/apichecker/apicheck.go b/cmd/apichecker/apicheck.go
@@ -1288,15 +1288,19 @@ func updateFile(name string) error {
 // SendGetReq sends get req
 func sendGetReq(path string, result interface{}) error {
 	var requester *request.Requester
+	var err error
 	if strings.Contains(path, "github") {
-		requester = request.New("Apichecker",
+		requester, err = request.New("Apichecker",
 			common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 			request.WithLimiter(request.NewBasicRateLimit(time.Hour, 60)))
 	} else {
-		requester = request.New("Apichecker",
+		requester, err = request.New("Apichecker",
 			common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 			request.WithLimiter(request.NewBasicRateLimit(time.Second, 100)))
 	}
+	if err != nil {
+		return err
+	}
 	item := &request.Item{
 		Method:  http.MethodGet,
 		Path:    path,
@@ -1309,9 +1313,12 @@ func sendGetReq(path string, result interface{}) error {
 
 // sendAuthReq sends auth req
 func sendAuthReq(method, path string, result interface{}) error {
-	requester := request.New("Apichecker",
+	requester, err := request.New("Apichecker",
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(request.NewBasicRateLimit(time.Second*10, 100)))
+	if err != nil {
+		return err
+	}
 	item := &request.Item{
 		Method:  method,
 		Path:    path,

diff --git a/currency/coinmarketcap/coinmarketcap.go b/currency/coinmarketcap/coinmarketcap.go
@@ -36,10 +36,14 @@ func (c *Coinmarketcap) SetDefaults() {
 	c.Verbose = false
 	c.APIUrl = baseURL
 	c.APIVersion = version
-	c.Requester = request.New(c.Name,
+	var err error
+	c.Requester, err = request.New(c.Name,
 		common.NewHTTPClientWithTimeout(defaultTimeOut),
 		request.WithLimiter(request.NewBasicRateLimit(RateInterval, BasicRequestRate)),
 	)
+	if err != nil {
+		log.Errorln(log.Global, err)
+	}
 }
 
 // Setup sets user configuration

diff --git a/currency/forexprovider/currencyconverterapi/currencyconverterapi.go b/currency/forexprovider/currencyconverterapi/currencyconverterapi.go
@@ -23,10 +23,11 @@ func (c *CurrencyConverter) Setup(config base.Settings) error {
 	c.Enabled = config.Enabled
 	c.Verbose = config.Verbose
 	c.PrimaryProvider = config.PrimaryProvider
-	c.Requester = request.New(c.Name,
+	var err error
+	c.Requester, err = request.New(c.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut),
 		request.WithLimiter(request.NewBasicRateLimit(rateInterval, requestRate)))
-	return nil
+	return err
 }
 
 // GetRates is a wrapper function to return rates

diff --git a/currency/forexprovider/currencylayer/currencylayer.go b/currency/forexprovider/currencylayer/currencylayer.go
@@ -43,10 +43,10 @@ func (c *CurrencyLayer) Setup(config base.Settings) error {
 	c.Verbose = config.Verbose
 	c.PrimaryProvider = config.PrimaryProvider
 	// Rate limit is based off a monthly counter - Open limit used.
-	c.Requester = request.New(c.Name,
+	var err error
+	c.Requester, err = request.New(c.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut))
-
-	return nil
+	return err
 }
 
 // GetRates is a wrapper function to return rates for GoCryptoTrader

diff --git a/currency/forexprovider/exchangerate.host/exchangerate.go b/currency/forexprovider/exchangerate.host/exchangerate.go
@@ -33,9 +33,10 @@ func (e *ExchangeRateHost) Setup(config base.Settings) error {
 	e.Enabled = config.Enabled
 	e.Verbose = config.Verbose
 	e.PrimaryProvider = config.PrimaryProvider
-	e.Requester = request.New(e.Name,
+	var err error
+	e.Requester, err = request.New(e.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut))
-	return nil
+	return err
 }
 
 // GetLatestRates returns a list of forex rates based on the supplied params

diff --git a/currency/forexprovider/exchangeratesapi.io/exchangeratesapi.go b/currency/forexprovider/exchangeratesapi.io/exchangeratesapi.go
@@ -29,10 +29,11 @@ func (e *ExchangeRates) Setup(config base.Settings) error {
 	e.PrimaryProvider = config.PrimaryProvider
 	e.APIKey = config.APIKey
 	e.APIKeyLvl = config.APIKeyLvl
-	e.Requester = request.New(e.Name,
+	var err error
+	e.Requester, err = request.New(e.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut),
 		request.WithLimiter(request.NewBasicRateLimit(rateLimitInterval, requestRate)))
-	return nil
+	return err
 }
 
 func (e *ExchangeRates) cleanCurrencies(baseCurrency, symbols string) string {

diff --git a/currency/forexprovider/fixer.io/fixer.go b/currency/forexprovider/fixer.io/fixer.go
@@ -36,9 +36,10 @@ func (f *Fixer) Setup(config base.Settings) error {
 	f.Name = config.Name
 	f.Verbose = config.Verbose
 	f.PrimaryProvider = config.PrimaryProvider
-	f.Requester = request.New(f.Name,
+	var err error
+	f.Requester, err = request.New(f.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut))
-	return nil
+	return err
 }
 
 // GetSupportedCurrencies returns supported currencies

diff --git a/currency/forexprovider/openexchangerates/openexchangerates.go b/currency/forexprovider/openexchangerates/openexchangerates.go
@@ -37,9 +37,10 @@ func (o *OXR) Setup(config base.Settings) error {
 	o.Name = config.Name
 	o.Verbose = config.Verbose
 	o.PrimaryProvider = config.PrimaryProvider
-	o.Requester = request.New(o.Name,
+	var err error
+	o.Requester, err = request.New(o.Name,
 		common.NewHTTPClientWithTimeout(base.DefaultTimeOut))
-	return nil
+	return err
 }
 
 // GetRates is a wrapper function to return rates

diff --git a/engine/exchange_manager.go b/engine/exchange_manager.go
@@ -94,12 +94,16 @@ func (m *ExchangeManager) RemoveExchange(exchName string) error {
 	if m.Len() == 0 {
 		return ErrNoExchangesLoaded
 	}
-	_, err := m.GetExchangeByName(exchName)
+	exch, err := m.GetExchangeByName(exchName)
 	if err != nil {
 		return err
 	}
 	m.m.Lock()
 	defer m.m.Unlock()
+	err = exch.GetBase().Requester.Shutdown()
+	if err != nil {
+		return err
+	}
 	delete(m.exchanges, strings.ToLower(exchName))
 	log.Infof(log.ExchangeSys, "%s exchange unloaded successfully.\n", exchName)
 	return nil

diff --git a/exchanges/alphapoint/alphapoint_wrapper.go b/exchanges/alphapoint/alphapoint_wrapper.go
@@ -73,8 +73,11 @@ func (a *Alphapoint) SetDefaults() {
 		},
 	}
 
-	a.Requester = request.New(a.Name,
+	a.Requester, err = request.New(a.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 }
 
 // FetchTradablePairs returns a list of the exchanges tradable pairs

diff --git a/exchanges/binance/binance_mock_test.go b/exchanges/binance/binance_mock_test.go
@@ -46,7 +46,10 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		log.Fatalf("Mock server error %s", err)
 	}
-	b.HTTPClient = newClient
+	err = b.SetHTTPClient(newClient)
+	if err != nil {
+		log.Fatalf("Mock server error %s", err)
+	}
 	endpointMap := b.API.Endpoints.GetURLMap()
 	for k := range endpointMap {
 		err = b.API.Endpoints.SetRunning(k, serverDetails)

diff --git a/exchanges/binance/binance_wrapper.go b/exchanges/binance/binance_wrapper.go
@@ -186,9 +186,12 @@ func (b *Binance) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:              spotAPIURL,

diff --git a/exchanges/bitfinex/bitfinex_wrapper.go b/exchanges/bitfinex/bitfinex_wrapper.go
@@ -165,9 +165,12 @@ func (b *Bitfinex) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      bitfinexAPIURLBase,

diff --git a/exchanges/bitflyer/bitflyer_wrapper.go b/exchanges/bitflyer/bitflyer_wrapper.go
@@ -94,9 +94,12 @@ func (b *Bitflyer) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      japanURL,

diff --git a/exchanges/bithumb/bithumb_wrapper.go b/exchanges/bithumb/bithumb_wrapper.go
@@ -129,9 +129,12 @@ func (b *Bithumb) SetDefaults() {
 			},
 		},
 	}
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      apiURL,

diff --git a/exchanges/bitmex/bitmex_wrapper.go b/exchanges/bitmex/bitmex_wrapper.go
@@ -124,9 +124,12 @@ func (b *Bitmex) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      bitmexAPIURL,

diff --git a/exchanges/bitstamp/bitstamp_mock_test.go b/exchanges/bitstamp/bitstamp_mock_test.go
@@ -46,7 +46,10 @@ func TestMain(m *testing.M) {
 		log.Fatalf("Mock server error %s", err)
 	}
 
-	b.HTTPClient = newClient
+	err = b.SetHTTPClient(newClient)
+	if err != nil {
+		log.Fatalf("Mock server error %s", err)
+	}
 	endpointMap := b.API.Endpoints.GetURLMap()
 	for k := range endpointMap {
 		err = b.API.Endpoints.SetRunning(k, serverDetails+"/api")

diff --git a/exchanges/bitstamp/bitstamp_wrapper.go b/exchanges/bitstamp/bitstamp_wrapper.go
@@ -129,9 +129,12 @@ func (b *Bitstamp) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(request.NewBasicRateLimit(bitstampRateInterval, bitstampRequestRate)))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      bitstampAPIURL,

diff --git a/exchanges/bittrex/bittrex_wrapper.go b/exchanges/bittrex/bittrex_wrapper.go
@@ -120,9 +120,12 @@ func (b *Bittrex) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(request.NewBasicRateLimit(ratePeriod, rateLimit)))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 
 	b.API.Endpoints = b.NewEndpoints()
 

diff --git a/exchanges/btcmarkets/btcmarkets_wrapper.go b/exchanges/btcmarkets/btcmarkets_wrapper.go
@@ -119,9 +119,12 @@ func (b *BTCMarkets) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      btcMarketsAPIURL,

diff --git a/exchanges/btse/btse_wrapper.go b/exchanges/btse/btse_wrapper.go
@@ -149,9 +149,12 @@ func (b *BTSE) SetDefaults() {
 		},
 	}
 
-	b.Requester = request.New(b.Name,
+	b.Requester, err = request.New(b.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	b.API.Endpoints = b.NewEndpoints()
 	err = b.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      btseAPIURL,

diff --git a/exchanges/coinbasepro/coinbasepro_wrapper.go b/exchanges/coinbasepro/coinbasepro_wrapper.go
@@ -130,9 +130,12 @@ func (c *CoinbasePro) SetDefaults() {
 		},
 	}
 
-	c.Requester = request.New(c.Name,
+	c.Requester, err = request.New(c.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout),
 		request.WithLimiter(SetRateLimit()))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	c.API.Endpoints = c.NewEndpoints()
 	err = c.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      coinbaseproAPIURL,

diff --git a/exchanges/coinut/coinut_wrapper.go b/exchanges/coinut/coinut_wrapper.go
@@ -113,8 +113,11 @@ func (c *COINUT) SetDefaults() {
 		},
 	}
 
-	c.Requester = request.New(c.Name,
+	c.Requester, err = request.New(c.Name,
 		common.NewHTTPClientWithTimeout(exchange.DefaultHTTPTimeout))
+	if err != nil {
+		log.Errorln(log.ExchangeSys, err)
+	}
 	c.API.Endpoints = c.NewEndpoints()
 	err = c.API.Endpoints.SetDefaultEndpoints(map[exchange.URL]string{
 		exchange.RestSpot:      coinutAPIURL,