Skip to content

Commit

Permalink
Kraken: Fix TestGetOpenInterest (#1611)
Browse files Browse the repository at this point in the history
* Kraken: Fix TestGetOpenInterest

We see daily failures on OpenInterest for Kraken.
This fix assumes that the issue might be related to volume of ETHUSD
open interest, and switches the single pair test to XBTUSD instead

Also isolates the test from others, since we're changing stored pairs
and we might be colliding on the global k

* Kraken: Handle Errors field in futures response
  • Loading branch information
gbjk authored Aug 16, 2024
1 parent 91ff6c5 commit 0becfbd
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 34 deletions.
1 change: 1 addition & 0 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ var (
ErrCannotCalculateOffline = errors.New("cannot calculate offline, unsupported")
ErrNoResponse = errors.New("no response")
ErrTypeAssertFailure = errors.New("type assert failure")
ErrUnknownError = errors.New("unknown error")
)

var (
Expand Down
2 changes: 1 addition & 1 deletion exchanges/binance/binance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2016,7 +2016,7 @@ func TestSubscribeBadResp(t *testing.T) {
}
b := testexch.MockWsInstance[Binance](t, testexch.CurryWsMockUpgrader(t, mock)) //nolint:govet // Intentional shadow to avoid future copy/paste mistakes
err := b.Subscribe(channels)
assert.ErrorIs(t, err, errUnknownError, "Subscribe should error errUnknownError")
assert.ErrorIs(t, err, common.ErrUnknownError, "Subscribe should error correctly")
assert.ErrorContains(t, err, "carrots", "Subscribe should error containing the carrots")
}

Expand Down
3 changes: 1 addition & 2 deletions exchanges/binance/binance_websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ var (
// maxWSOrderbookWorkers defines a max amount of workers allowed to execute
// jobs from the job channel
maxWSOrderbookWorkers = 10
errUnknownError = errors.New("unknown error")
)

// WsConnect initiates a websocket connection
Expand Down Expand Up @@ -584,7 +583,7 @@ func (b *Binance) manageSubs(op string, subs subscription.List) error {
if v, d, _, rErr := jsonparser.Get(respRaw, "result"); rErr != nil {
err = rErr
} else if d != jsonparser.Null { // null is the only expected and acceptable response
err = fmt.Errorf("%w: %s", errUnknownError, v)
err = fmt.Errorf("%w: %s", common.ErrUnknownError, v)
}
}

Expand Down
2 changes: 1 addition & 1 deletion exchanges/bitfinex/bitfinex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1979,7 +1979,7 @@ func TestGetErrResp(t *testing.T) {
case 3: // event != 'error'
assert.NoError(t, testErr, "Message with non-'error' event field should not error")
case 4: // event="error"
assert.ErrorIs(t, testErr, errUnknownError, "error without a message should throw unknown error")
assert.ErrorIs(t, testErr, common.ErrUnknownError, "error without a message should throw unknown error")
assert.ErrorContains(t, testErr, "code: 0", "error without a code should throw code 0")
case 5: // Fully formatted
assert.ErrorContains(t, testErr, "redcoats", "message field should be in the error")
Expand Down
1 change: 0 additions & 1 deletion exchanges/bitfinex/bitfinex_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
var (
errSetCannotBeEmpty = errors.New("set cannot be empty")
errNoSeqNo = errors.New("no sequence number")
errUnknownError = errors.New("unknown error")
errParamNotAllowed = errors.New("param not allowed")
errParsingWSField = errors.New("error parsing WS field")
errTickerInvalidSymbol = errors.New("invalid ticker symbol")
Expand Down
4 changes: 2 additions & 2 deletions exchanges/bitfinex/bitfinex_websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ func (b *Bitfinex) unsubscribeFromChan(chans subscription.List) error {
// getErrResp takes a json response string and looks for an error event type
// If found it parses the error code and message as a wrapped error and returns it
// It might log parsing errors about the nature of the error
// If the error message is not defined it will return a wrapped errUnknownError
// If the error message is not defined it will return a wrapped common.ErrUnknownError
func (b *Bitfinex) getErrResp(resp []byte) error {
event, err := jsonparser.GetUnsafeString(resp, "event")
if err != nil {
Expand All @@ -1883,7 +1883,7 @@ func (b *Bitfinex) getErrResp(resp []byte) error {
var apiErr error
if msg, e2 := jsonparser.GetString(resp, "msg"); e2 != nil {
log.Errorf(log.ExchangeSys, "%s %s 'msg': %s from message: %s", b.Name, errParsingWSField, e2, resp)
apiErr = errUnknownError
apiErr = common.ErrUnknownError
} else {
apiErr = errors.New(msg)
}
Expand Down
7 changes: 1 addition & 6 deletions exchanges/kraken/kraken.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,15 +848,10 @@ func (k *Kraken) SendHTTPRequest(ctx context.Context, ep exchange.URL, path stri
return genResponse.Error.Errors()
}

var genResp genericFuturesResponse
if err := json.Unmarshal(rawMessage, &genResp); err != nil {
if err := getFuturesErr(rawMessage); err != nil {
return err
}

if genResp.Error != "" && genResp.Result != "success" {
return errors.New(genResp.Error)
}

return json.Unmarshal(rawMessage, result)
}

Expand Down
42 changes: 34 additions & 8 deletions exchanges/kraken/kraken_futures.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,20 +377,46 @@ func (k *Kraken) SendFuturesAuthRequest(ctx context.Context, method, path string
}, nil
}

if err := k.SendPayload(ctx, request.Unset, newRequest, request.AuthenticatedRequest); err != nil {
return err
err = k.SendPayload(ctx, request.Unset, newRequest, request.AuthenticatedRequest)

if err == nil {
err = getFuturesErr(interim)
}

var resp genericFuturesResponse
if err := json.Unmarshal(interim, &resp); err != nil {
return fmt.Errorf("%w %w", request.ErrAuthRequestFailed, err)
} else if resp.Error != "" && resp.Result != "success" {
return fmt.Errorf("%w %v", request.ErrAuthRequestFailed, resp.Error)
if err == nil {
err = json.Unmarshal(interim, result)
}

if err := json.Unmarshal(interim, result); err != nil {
if err != nil {
return fmt.Errorf("%w %w", request.ErrAuthRequestFailed, err)
}

return nil
}

func getFuturesErr(msg json.RawMessage) error {
var resp genericFuturesResponse
if err := json.Unmarshal(msg, &resp); err != nil {
return err
}

// Result may be omitted entirely, so we don't test for == "success"
if resp.Result != "error" {
return nil
}

var errs error
if resp.Error != "" {
errs = errors.New(resp.Error)
}

for _, err := range resp.Errors {
errs = common.AppendError(errs, errors.New(err))
}

if errs == nil {
return fmt.Errorf("%w from message: %s", common.ErrUnknownError, msg)
}

return errs
}
37 changes: 26 additions & 11 deletions exchanges/kraken/kraken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2079,20 +2079,15 @@ func TestChecksumCalculation(t *testing.T) {
func TestGetCharts(t *testing.T) {
t.Parallel()
cp, err := currency.NewPairFromStrings("PI", "BCHUSD")
if err != nil {
t.Error(err)
}
require.NoError(t, err)
cp.Delimiter = "_"
resp, err := k.GetFuturesCharts(context.Background(), "1d", "spot", cp, time.Time{}, time.Time{})
if err != nil {
t.Error(err)
}
require.NoError(t, err)
require.NotEmpty(t, resp.Candles)

end := time.UnixMilli(resp.Candles[0].Time)
_, err = k.GetFuturesCharts(context.Background(), "1d", "spot", cp, end.Add(-time.Hour*24*7), end)
if err != nil {
t.Error(err)
}
require.NoError(t, err)
}

func TestGetFuturesTrades(t *testing.T) {
Expand Down Expand Up @@ -2251,15 +2246,18 @@ func TestIsPerpetualFutureCurrency(t *testing.T) {

func TestGetOpenInterest(t *testing.T) {
t.Parallel()
k := new(Kraken) //nolint:govet // Intentional shadow to avoid future copy/paste mistakes
require.NoError(t, testexch.Setup(k), "Test instance Setup must not error")

_, err := k.GetOpenInterest(context.Background(), key.PairAsset{
Base: currency.ETH.Item,
Quote: currency.USDT.Item,
Asset: asset.USDTMarginedFutures,
})
assert.ErrorIs(t, err, asset.ErrNotSupported)

cp1 := currency.NewPair(currency.PF, currency.NewCode("ETHUSD"))
cp2 := currency.NewPair(currency.PF, currency.NewCode("XBTUSD"))
cp1 := currency.NewPair(currency.PF, currency.NewCode("XBTUSD"))
cp2 := currency.NewPair(currency.PF, currency.NewCode("ETHUSD"))
sharedtestvalues.SetupCurrencyPairsForExchangeAsset(t, k, asset.Futures, cp1, cp2)

resp, err := k.GetOpenInterest(context.Background(), key.PairAsset{
Expand Down Expand Up @@ -2396,3 +2394,20 @@ func TestErrorResponse(t *testing.T) {
})
}
}

func TestGetFuturesErr(t *testing.T) {
t.Parallel()

assert.ErrorContains(t, getFuturesErr(json.RawMessage(`unparsable rubbish`)), "invalid character", "Bad JSON should error correctly")
assert.NoError(t, getFuturesErr(json.RawMessage(`{"candles":[]}`)), "JSON with no Result should not error")
assert.NoError(t, getFuturesErr(json.RawMessage(`{"Result":"4 goats"}`)), "JSON with non-error Result should not error")
assert.ErrorIs(t, getFuturesErr(json.RawMessage(`{"Result":"error"}`)), common.ErrUnknownError, "JSON with error Result should error correctly")
assert.ErrorContains(t, getFuturesErr(json.RawMessage(`{"Result":"error", "error": "1 goat"}`)), "1 goat", "JSON with an error should error correctly")
err := getFuturesErr(json.RawMessage(`{"Result":"error", "errors": ["2 goats", "3 goats"]}`))
assert.ErrorContains(t, err, "2 goat", "JSON with errors should error correctly")
assert.ErrorContains(t, err, "3 goat", "JSON with errors should error correctly")
err = getFuturesErr(json.RawMessage(`{"Result":"error", "error": "too many goats", "errors": ["2 goats", "3 goats"]}`))
assert.ErrorContains(t, err, "2 goat", "JSON with both error and errors should error correctly")
assert.ErrorContains(t, err, "3 goat", "JSON with both error and errors should error correctly")
assert.ErrorContains(t, err, "too many goat", "JSON both error and with errors should error correctly")
}
1 change: 1 addition & 0 deletions exchanges/kraken/kraken_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type genericFuturesResponse struct {
Result string `json:"result"`
ServerTime time.Time `json:"serverTime"`
Error string `json:"error"`
Errors []string `json:"errors"`
}

// Asset holds asset information
Expand Down
3 changes: 1 addition & 2 deletions exchanges/kraken/kraken_websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ var (
m sync.Mutex
errNoWebsocketOrderbookData = errors.New("no websocket orderbook data")
errParsingWSField = errors.New("error parsing WS field")
errUnknownError = errors.New("unknown error")
errCancellingOrder = errors.New("error cancelling order")
)

Expand Down Expand Up @@ -1360,7 +1359,7 @@ func (k *Kraken) wsCancelOrder(orderID string) error {
return nil
}

err = errUnknownError
err = common.ErrUnknownError
if msg, pErr := jsonparser.GetUnsafeString(resp, "errorMessage"); pErr == nil && msg != "" {
err = errors.New(msg)
}
Expand Down

0 comments on commit 0becfbd

Please sign in to comment.