Engine/ExchangeManager: Return error for method GetExchangeByName (th…

…rasher-corp#760)

* engine: Add error returns

* engine: after merge fixes

* engine: remove interface

* linter: fix shadow declarations

* engine: fix tests

* niterinos: fixed

* GLORIOUS NITS!

backtester/backtest/backtest.go

@@ -135,7 +135,11 @@ func NewFromConfig(cfg *config.Config, templatePath, output string, bot *engine.
 				cfg.CurrencySettings[i].Base+cfg.CurrencySettings[i].Quote,
 				err)
 		}
-		exch := bot.ExchangeManager.GetExchangeByName(cfg.CurrencySettings[i].ExchangeName)
+		var exch gctexchange.IBotExchange
+		exch, err = bot.ExchangeManager.GetExchangeByName(cfg.CurrencySettings[i].ExchangeName)
+		if err != nil {
+			return nil, fmt.Errorf("could not get exchange by name %w", err)
+		}
 		b := exch.GetBase()
 		var pFmt currency.PairFormat
 		pFmt, err = b.GetPairFormat(a, true)
@@ -332,10 +336,9 @@ func (bt *BackTest) setupExchangeSettings(cfg *config.Config) (exchange.Exchange
 }
 
 func (bt *BackTest) loadExchangePairAssetBase(exch, base, quote, ass string) (gctexchange.IBotExchange, currency.Pair, asset.Item, error) {
-	var err error
-	e := bt.Bot.GetExchangeByName(exch)
-	if e == nil {
-		return nil, currency.Pair{}, "", engine.ErrExchangeNotFound
+	e, err := bt.Bot.GetExchangeByName(exch)
+	if err != nil {
+		return nil, currency.Pair{}, "", err
 	}
 
 	var cp, fPair currency.Pair

engine/apiserver.go

@@ -19,7 +19,6 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/common/crypto"
 	"github.com/thrasher-corp/gocryptotrader/config"
 	"github.com/thrasher-corp/gocryptotrader/currency"
-	"github.com/thrasher-corp/gocryptotrader/database/repository/exchange"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
 	"github.com/thrasher-corp/gocryptotrader/log"
 )
@@ -805,9 +804,9 @@ func wsGetTicker(client *websocketClient, data interface{}) error {
 		return err
 	}
 
-	exch := client.exchangeManager.GetExchangeByName(tickerReq.Exchange)
-	if exch == nil {
-		wsResp.Error = exchange.ErrNoExchangeFound.Error()
+	exch, err := client.exchangeManager.GetExchangeByName(tickerReq.Exchange)
+	if err != nil {
+		wsResp.Error = err.Error()
 		sendErr := client.SendWebsocketMessage(wsResp)
 		if sendErr != nil {
 			log.Error(log.APIServerMgr, sendErr)
@@ -860,9 +859,9 @@ func wsGetOrderbook(client *websocketClient, data interface{}) error {
 		return err
 	}
 
-	exch := client.exchangeManager.GetExchangeByName(orderbookReq.Exchange)
-	if exch == nil {
-		wsResp.Error = exchange.ErrNoExchangeFound.Error()
+	exch, err := client.exchangeManager.GetExchangeByName(orderbookReq.Exchange)
+	if err != nil {
+		wsResp.Error = err.Error()
 		sendErr := client.SendWebsocketMessage(wsResp)
 		if sendErr != nil {
 			log.Error(log.APIServerMgr, sendErr)

engine/datahistory_manager.go

@@ -302,31 +302,30 @@ func (m *DataHistoryManager) runJob(job *DataHistoryJob) error {
 	if job.DataType == dataHistoryCandleValidationSecondarySourceType {
 		exchangeName = job.SecondaryExchangeSource
 	}
-	exch := m.exchangeManager.GetExchangeByName(exchangeName)
-	if exch == nil {
-		return fmt.Errorf("%s %w, cannot process job %s for %s %s",
-			job.Exchange,
-			errExchangeNotLoaded,
+	exch, err := m.exchangeManager.GetExchangeByName(exchangeName)
+	if err != nil {
+		return fmt.Errorf("%w, cannot process job %s for %s %s",
+			err,
 			job.Nickname,
 			job.Asset,
 			job.Pair)
 	}
 
 	if job.DataType == dataHistoryCandleValidationDataType ||
 		job.DataType == dataHistoryCandleValidationSecondarySourceType {
-		err := m.runValidationJob(job, exch)
+		err = m.runValidationJob(job, exch)
 		if err != nil {
 			return err
 		}
 	} else {
-		err := m.runDataJob(job, exch)
+		err = m.runDataJob(job, exch)
 		if err != nil {
 			return err
 		}
 	}
 
 	dbJob := m.convertJobToDBModel(job)
-	err := m.jobDB.Upsert(dbJob)
+	err = m.jobDB.Upsert(dbJob)
 	if err != nil {
 		return fmt.Errorf("job %s failed to update database: %w", job.Nickname, err)
 	}
@@ -1172,12 +1171,9 @@ func (m *DataHistoryManager) validateJob(job *DataHistoryJob) error {
 			return fmt.Errorf("job %s %w, exchange name required to lookup existing results", job.Nickname, errExchangeNameUnset)
 		}
 	}
-	exch := m.exchangeManager.GetExchangeByName(exchangeName)
-	if exch == nil {
-		return fmt.Errorf("job %s cannot process job: %s %w",
-			job.Nickname,
-			job.Exchange,
-			errExchangeNotLoaded)
+	exch, err := m.exchangeManager.GetExchangeByName(exchangeName)
+	if err != nil {
+		return fmt.Errorf("job %s cannot process job: %v", job.Nickname, err)
 	}
 	pairs, err := exch.GetEnabledPairs(job.Asset)
 	if err != nil {

engine/engine.go

@@ -681,7 +681,7 @@ func (bot *Engine) Stop() {
 }
 
 // GetExchangeByName returns an exchange given an exchange name
-func (bot *Engine) GetExchangeByName(exchName string) exchange.IBotExchange {
+func (bot *Engine) GetExchangeByName(exchName string) (exchange.IBotExchange, error) {
 	return bot.ExchangeManager.GetExchangeByName(exchName)
 }
 

engine/engine_test.go

@@ -165,27 +165,13 @@ func TestStartStopTwoDoesNotCausePanic(t *testing.T) {
 	botTwo.Stop()
 }
 
-func TestCheckExchangeExists(t *testing.T) {
+func TestGetExchangeByName(t *testing.T) {
 	t.Parallel()
-	em := SetupExchangeManager()
-	exch, err := em.NewExchangeByName(testExchange)
-	if !errors.Is(err, nil) {
-		t.Fatalf("received '%v' expected '%v'", err, nil)
-	}
-	exch.SetDefaults()
-	em.Add(exch)
-	e := &Engine{ExchangeManager: em}
-	if e.GetExchangeByName(testExchange) == nil {
-		t.Errorf("TestGetExchangeExists: Unable to find exchange")
+	_, err := (*ExchangeManager)(nil).GetExchangeByName("tehehe")
+	if !errors.Is(err, ErrNilSubsystem) {
+		t.Errorf("received: %v expected: %v", err, ErrNilSubsystem)
 	}
 
-	if e.GetExchangeByName("Asdsad") != nil {
-		t.Errorf("TestGetExchangeExists: Non-existent exchange found")
-	}
-}
-
-func TestGetExchangeByName(t *testing.T) {
-	t.Parallel()
 	em := SetupExchangeManager()
 	exch, err := em.NewExchangeByName(testExchange)
 	if !errors.Is(err, nil) {
@@ -201,18 +187,20 @@ func TestGetExchangeByName(t *testing.T) {
 	}
 
 	exch.SetEnabled(false)
-	bfx := e.GetExchangeByName(testExchange)
+	bfx, err := e.GetExchangeByName(testExchange)
+	if err != nil {
+		t.Fatal(err)
+	}
 	if bfx.IsEnabled() {
 		t.Errorf("TestGetExchangeByName: Unexpected result")
 	}
-
 	if exch.GetName() != testExchange {
 		t.Errorf("TestGetExchangeByName: Unexpected result")
 	}
 
-	exch = e.GetExchangeByName("Asdasd")
-	if exch != nil {
-		t.Errorf("TestGetExchangeByName: Non-existent exchange found")
+	_, err = e.GetExchangeByName("Asdasd")
+	if !errors.Is(err, ErrExchangeNotFound) {
+		t.Errorf("received: %v expected: %v", err, ErrExchangeNotFound)
 	}
 }
 

engine/event_manager.go

@@ -234,7 +234,8 @@ func (m *eventManager) isValidEvent(exchange, item string, condition EventCondit
 
 // isValidExchange validates the exchange
 func (m *eventManager) isValidExchange(exchangeName string) bool {
-	return m.exchangeManager.GetExchangeByName(exchangeName) != nil
+	_, err := m.exchangeManager.GetExchangeByName(exchangeName)
+	return err == nil
 }
 
 // isValidCondition validates passed in condition

engine/exchange_manager.go

@@ -43,6 +43,7 @@ var (
 	ErrExchangeNotFound      = errors.New("exchange not found")
 	ErrExchangeAlreadyLoaded = errors.New("exchange already loaded")
 	ErrExchangeFailedToLoad  = errors.New("exchange failed to load")
+	errExchangeNameIsEmpty   = errors.New("exchange name is empty")
 )
 
 // CustomExchangeBuilder interface allows external applications to create
@@ -91,9 +92,9 @@ func (m *ExchangeManager) RemoveExchange(exchName string) error {
 	if m.Len() == 0 {
 		return ErrNoExchangesLoaded
 	}
-	exch := m.GetExchangeByName(exchName)
-	if exch == nil {
-		return ErrExchangeNotFound
+	_, err := m.GetExchangeByName(exchName)
+	if err != nil {
+		return err
 	}
 	m.m.Lock()
 	defer m.m.Unlock()
@@ -103,17 +104,20 @@ func (m *ExchangeManager) RemoveExchange(exchName string) error {
 }
 
 // GetExchangeByName returns an exchange by its name if it exists
-func (m *ExchangeManager) GetExchangeByName(exchangeName string) exchange.IBotExchange {
+func (m *ExchangeManager) GetExchangeByName(exchangeName string) (exchange.IBotExchange, error) {
 	if m == nil {
-		return nil
+		return nil, fmt.Errorf("exchange manager: %w", ErrNilSubsystem)
+	}
+	if exchangeName == "" {
+		return nil, fmt.Errorf("exchange manager: %w", errExchangeNameIsEmpty)
 	}
 	m.m.Lock()
 	defer m.m.Unlock()
 	exch, ok := m.exchanges[strings.ToLower(exchangeName)]
 	if !ok {
-		return nil
+		return nil, fmt.Errorf("%s %w", exchangeName, ErrExchangeNotFound)
 	}
-	return exch
+	return exch, nil
 }
 
 // Len says how many exchanges are loaded
@@ -129,7 +133,7 @@ func (m *ExchangeManager) NewExchangeByName(name string) (exchange.IBotExchange,
 		return nil, fmt.Errorf("exchange manager %w", ErrNilSubsystem)
 	}
 	nameLower := strings.ToLower(name)
-	if m.GetExchangeByName(nameLower) != nil {
+	if exch, _ := m.GetExchangeByName(nameLower); exch != nil {
 		return nil, fmt.Errorf("%s %w", name, ErrExchangeAlreadyLoaded)
 	}
 	var exch exchange.IBotExchange

engine/exchange_manager_test.go

@@ -1,6 +1,7 @@
 package engine
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"testing"
@@ -55,8 +56,9 @@ func TestExchangeManagerRemoveExchange(t *testing.T) {
 	b := new(bitfinex.Bitfinex)
 	b.SetDefaults()
 	m.Add(b)
-	if err := m.RemoveExchange("Bitstamp"); err != ErrExchangeNotFound {
-		t.Error("Bitstamp exchange should return an error")
+	err := m.RemoveExchange("Bitstamp")
+	if !errors.Is(err, ErrExchangeNotFound) {
+		t.Errorf("received: %v but expected: %v", err, ErrExchangeNotFound)
 	}
 	if err := m.RemoveExchange("BiTFiNeX"); err != nil {
 		t.Error("exchange should have been removed")

engine/helpers.go

@@ -531,19 +531,19 @@ func GetRelatableCurrencies(p currency.Pair, incOrig, incUSDT bool) currency.Pai
 // GetSpecificOrderbook returns a specific orderbook given the currency,
 // exchangeName and assetType
 func (bot *Engine) GetSpecificOrderbook(p currency.Pair, exchangeName string, assetType asset.Item) (*orderbook.Base, error) {
-	exch := bot.GetExchangeByName(exchangeName)
-	if exch == nil {
-		return nil, ErrExchangeNotFound
+	exch, err := bot.GetExchangeByName(exchangeName)
+	if err != nil {
+		return nil, err
 	}
 	return exch.FetchOrderbook(p, assetType)
 }
 
 // GetSpecificTicker returns a specific ticker given the currency,
 // exchangeName and assetType
 func (bot *Engine) GetSpecificTicker(p currency.Pair, exchangeName string, assetType asset.Item) (*ticker.Price, error) {
-	exch := bot.GetExchangeByName(exchangeName)
-	if exch == nil {
-		return nil, ErrExchangeNotFound
+	exch, err := bot.GetExchangeByName(exchangeName)
+	if err != nil {
+		return nil, err
 	}
 	return exch.FetchTicker(p, assetType)
 }
@@ -661,9 +661,9 @@ func (bot *Engine) GetExchangeCryptocurrencyDepositAddress(exchName, accountID s
 		return bot.DepositAddressManager.GetDepositAddressByExchangeAndCurrency(exchName, item)
 	}
 
-	exch := bot.GetExchangeByName(exchName)
-	if exch == nil {
-		return "", ErrExchangeNotFound
+	exch, err := bot.GetExchangeByName(exchName)
+	if err != nil {
+		return "", err
 	}
 	return exch.GetDepositAddress(item, accountID)
 }

engine/helpers_test.go

@@ -162,7 +162,11 @@ func TestGetAuthAPISupportedExchanges(t *testing.T) {
 		t.Fatal("Unexpected result", result)
 	}
 
-	exch := e.ExchangeManager.GetExchangeByName(testExchange)
+	exch, err := e.ExchangeManager.GetExchangeByName(testExchange)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	b := exch.GetBase()
 	b.API.AuthenticatedWebsocketSupport = true
 	b.API.Credentials.Key = "test"