From f2c3b37e0d8e4f9304d11b27d91431898692d635 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 08:33:58 -0700 Subject: [PATCH 1/7] Return correct error when context cancelled --- fetcher/account.go | 6 +++++- fetcher/block.go | 6 +++++- fetcher/network.go | 18 +++++++++++++++--- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/fetcher/account.go b/fetcher/account.go index 3d60e1b6..fa1b139c 100644 --- a/fetcher/account.go +++ b/fetcher/account.go @@ -71,7 +71,7 @@ func (f *Fetcher) AccountBalanceRetry( f.maxRetries, ) - for ctx.Err() == nil { + for { responseBlock, balances, metadata, err := f.AccountBalance( ctx, network, @@ -82,6 +82,10 @@ func (f *Fetcher) AccountBalanceRetry( return responseBlock, balances, metadata, nil } + if ctx.Err() != nil { + return nil, nil, nil, ctx.Err() + } + if !tryAgain(fmt.Sprintf("account %s", account.Address), backoffRetries, err) { break } diff --git a/fetcher/block.go b/fetcher/block.go index ca2281ce..6aa6e070 100644 --- a/fetcher/block.go +++ b/fetcher/block.go @@ -201,7 +201,7 @@ func (f *Fetcher) BlockRetry( f.maxRetries, ) - for ctx.Err() == nil { + for { block, err := f.Block( ctx, network, @@ -211,6 +211,10 @@ func (f *Fetcher) BlockRetry( return block, nil } + if ctx.Err() != nil { + return nil, ctx.Err() + } + var blockFetchErr string if blockIdentifier.Index != nil { blockFetchErr = fmt.Sprintf("block %d", *blockIdentifier.Index) diff --git a/fetcher/network.go b/fetcher/network.go index 9b94a332..2ac4f9d9 100644 --- a/fetcher/network.go +++ b/fetcher/network.go @@ -60,7 +60,7 @@ func (f *Fetcher) NetworkStatusRetry( f.maxRetries, ) - for ctx.Err() == nil { + for { networkStatus, err := f.NetworkStatus( ctx, network, @@ -70,6 +70,10 @@ func (f *Fetcher) NetworkStatusRetry( return networkStatus, nil } + if ctx.Err() != nil { + return nil, ctx.Err() + } + if !tryAgain("NetworkStatus", backoffRetries, err) { break } @@ -112,7 +116,7 @@ func (f *Fetcher) NetworkListRetry( f.maxRetries, ) - for ctx.Err() == nil { + for { networkList, err := f.NetworkList( ctx, metadata, @@ -121,6 +125,10 @@ func (f *Fetcher) NetworkListRetry( return networkList, nil } + if ctx.Err() != nil { + return nil, ctx.Err() + } + if !tryAgain("NetworkList", backoffRetries, err) { break } @@ -166,7 +174,7 @@ func (f *Fetcher) NetworkOptionsRetry( f.maxRetries, ) - for ctx.Err() == nil { + for { networkOptions, err := f.NetworkOptions( ctx, network, @@ -176,6 +184,10 @@ func (f *Fetcher) NetworkOptionsRetry( return networkOptions, nil } + if ctx.Err() != nil { + return nil, ctx.Err() + } + if !tryAgain("NetworkOptions", backoffRetries, err) { break } From 5cd576643b92a0d788d29becbc6b3180040ca465 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 08:49:48 -0700 Subject: [PATCH 2/7] Return correct error in reconciler --- reconciler/reconciler.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/reconciler/reconciler.go b/reconciler/reconciler.go index 42a63209..f23b0759 100644 --- a/reconciler/reconciler.go +++ b/reconciler/reconciler.go @@ -416,7 +416,11 @@ func (r *Reconciler) accountReconciliation( Account: account, Currency: currency, } - for ctx.Err() == nil { + for { + if ctx.Err() != nil { + return ctx.Err() + } + // If don't have previous balance because stateless, check diff on block // instead of comparing entire computed balance difference, cachedBalance, headIndex, err := r.CompareBalance( @@ -580,7 +584,11 @@ func (r *Reconciler) reconcileActiveAccounts( func (r *Reconciler) reconcileInactiveAccounts( ctx context.Context, ) error { - for ctx.Err() == nil { + for { + if ctx.Err() != nil { + return ctx.Err() + } + head, err := r.helper.CurrentBlock(ctx) // When first start syncing, this loop may run before the genesis block is synced. // If this is the case, we should sleep and try again later instead of exiting. @@ -624,8 +632,6 @@ func (r *Reconciler) reconcileInactiveAccounts( time.Sleep(inactiveReconciliationSleep) } } - - return nil } // Reconcile starts the active and inactive Reconciler goroutines. From 71731e550450e91034354dd9418705374444e34b Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 17:25:05 -0700 Subject: [PATCH 3/7] Adding coverage to fetcher --- fetcher/account_test.go | 124 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 fetcher/account_test.go diff --git a/fetcher/account_test.go b/fetcher/account_test.go new file mode 100644 index 00000000..dba3f8fc --- /dev/null +++ b/fetcher/account_test.go @@ -0,0 +1,124 @@ +package fetcher + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/coinbase/rosetta-sdk-go/types" + + "github.com/stretchr/testify/assert" +) + +var ( + basicNetwork = &types.NetworkIdentifier{ + Blockchain: "blockchain", + Network: "network", + } + + basicAccount = &types.AccountIdentifier{ + Address: "address", + } + + basicBlock = &types.BlockIdentifier{ + Index: 10, + Hash: "block 10", + } + + basicPartialBlock = &types.PartialBlockIdentifier{ + Index: &basicBlock.Index, + } + + basicAmounts = []*types.Amount{ + { + Value: "1000", + Currency: &types.Currency{ + Symbol: "BTC", + Decimals: 8, + }, + }, + } +) + +func TestAccountBalanceRetry(t *testing.T) { + var tests = map[string]struct { + network *types.NetworkIdentifier + account *types.AccountIdentifier + block *types.PartialBlockIdentifier + + errorsBeforeSuccess int + expectedBlock *types.BlockIdentifier + expectedAmounts []*types.Amount + expectedMetadata map[string]interface{} + expectedError error + + fetcherMaxRetries uint64 + shouldCancel bool + }{ + "single failure": { + network: basicNetwork, + account: basicAccount, + block: nil, + errorsBeforeSuccess: 1, + expectedBlock: basicBlock, + expectedAmounts: basicAmounts, + expectedError: nil, + fetcherMaxRetries: 5, + shouldCancel: false, + }, + } + for name, test := range tests { + runs := 0 + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal("POST", r.Method) + assert.Equal("/account/balance", r.URL.RequestURI()) + + expected := &types.AccountBalanceRequest{ + NetworkIdentifier: test.network, + AccountIdentifier: test.account, + BlockIdentifier: test.block, + } + var accountRequest *types.AccountBalanceRequest + assert.NoError(json.NewDecoder(r.Body).Decode(&accountRequest)) + assert.Equal(expected, accountRequest) + if runs < test.errorsBeforeSuccess { + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "{}") + runs++ + return + } + + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, types.PrettyPrintStruct( + &types.AccountBalanceResponse{ + BlockIdentifier: test.expectedBlock, + Balances: test.expectedAmounts, + Metadata: test.expectedMetadata, + }, + )) + })) + + defer ts.Close() + + f := New(ts.URL, WithRetryElapsedTime(time.Second), WithMaxRetries(test.fetcherMaxRetries)) + block, amounts, metadata, err := f.AccountBalanceRetry( + context.Background(), + test.network, + test.account, + test.block, + ) + assert.Equal(test.expectedBlock, block) + assert.Equal(test.expectedAmounts, amounts) + assert.Equal(test.expectedMetadata, metadata) + assert.Equal(test.expectedError, err) + }) + } +} From 8d4e1d24a70fb1fd83392a6397553897ba5d94de Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 17:44:50 -0700 Subject: [PATCH 4/7] test retries in account --- fetcher/account.go | 9 ++++-- fetcher/account_test.go | 64 +++++++++++++++++++++++++++++------------ fetcher/fetcher.go | 6 ++++ fetcher/utils.go | 2 ++ 4 files changed, 59 insertions(+), 22 deletions(-) diff --git a/fetcher/account.go b/fetcher/account.go index fa1b139c..97921f52 100644 --- a/fetcher/account.go +++ b/fetcher/account.go @@ -16,7 +16,6 @@ package fetcher import ( "context" - "errors" "fmt" "github.com/coinbase/rosetta-sdk-go/asserter" @@ -86,10 +85,14 @@ func (f *Fetcher) AccountBalanceRetry( return nil, nil, nil, ctx.Err() } - if !tryAgain(fmt.Sprintf("account %s", account.Address), backoffRetries, err) { + if !tryAgain(fmt.Sprintf("account %s", types.PrettyPrintStruct(account)), backoffRetries, err) { break } } - return nil, nil, nil, errors.New("exhausted retries for account") + return nil, nil, nil, fmt.Errorf( + "%w: unable to fetch account %s", + ErrExhaustedRetries, + types.PrettyPrintStruct(account), + ) } diff --git a/fetcher/account_test.go b/fetcher/account_test.go index dba3f8fc..f988a21a 100644 --- a/fetcher/account_test.go +++ b/fetcher/account_test.go @@ -3,6 +3,7 @@ package fetcher import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -48,50 +49,76 @@ func TestAccountBalanceRetry(t *testing.T) { var tests = map[string]struct { network *types.NetworkIdentifier account *types.AccountIdentifier - block *types.PartialBlockIdentifier errorsBeforeSuccess int expectedBlock *types.BlockIdentifier expectedAmounts []*types.Amount - expectedMetadata map[string]interface{} expectedError error fetcherMaxRetries uint64 shouldCancel bool }{ - "single failure": { + "no failures": { + network: basicNetwork, + account: basicAccount, + expectedBlock: basicBlock, + expectedAmounts: basicAmounts, + fetcherMaxRetries: 5, + }, + "retry failures": { network: basicNetwork, account: basicAccount, - block: nil, - errorsBeforeSuccess: 1, + errorsBeforeSuccess: 2, expectedBlock: basicBlock, expectedAmounts: basicAmounts, - expectedError: nil, fetcherMaxRetries: 5, - shouldCancel: false, + }, + "exhausted retries": { + network: basicNetwork, + account: basicAccount, + errorsBeforeSuccess: 2, + expectedError: ErrExhaustedRetries, + fetcherMaxRetries: 1, + }, + "cancel context": { + network: basicNetwork, + account: basicAccount, + errorsBeforeSuccess: 6, + expectedError: context.Canceled, + fetcherMaxRetries: 5, + shouldCancel: true, }, } + for name, test := range tests { - runs := 0 t.Run(name, func(t *testing.T) { - assert := assert.New(t) + var ( + tries = 0 + assert = assert.New(t) + ctx, cancel = context.WithCancel(context.Background()) + endpoint = "/account/balance" + ) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal("POST", r.Method) - assert.Equal("/account/balance", r.URL.RequestURI()) + assert.Equal(endpoint, r.URL.RequestURI()) expected := &types.AccountBalanceRequest{ NetworkIdentifier: test.network, AccountIdentifier: test.account, - BlockIdentifier: test.block, } var accountRequest *types.AccountBalanceRequest assert.NoError(json.NewDecoder(r.Body).Decode(&accountRequest)) assert.Equal(expected, accountRequest) - if runs < test.errorsBeforeSuccess { + + if test.shouldCancel { + cancel() + } + + if tries < test.errorsBeforeSuccess { w.Header().Set("Content-Type", "application/json; charset=UTF-8") w.WriteHeader(http.StatusInternalServerError) fmt.Fprintln(w, "{}") - runs++ + tries++ return } @@ -101,24 +128,23 @@ func TestAccountBalanceRetry(t *testing.T) { &types.AccountBalanceResponse{ BlockIdentifier: test.expectedBlock, Balances: test.expectedAmounts, - Metadata: test.expectedMetadata, }, )) })) defer ts.Close() - f := New(ts.URL, WithRetryElapsedTime(time.Second), WithMaxRetries(test.fetcherMaxRetries)) + f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) block, amounts, metadata, err := f.AccountBalanceRetry( - context.Background(), + ctx, test.network, test.account, - test.block, + nil, ) assert.Equal(test.expectedBlock, block) assert.Equal(test.expectedAmounts, amounts) - assert.Equal(test.expectedMetadata, metadata) - assert.Equal(test.expectedError, err) + assert.Nil(metadata) + assert.True(errors.Is(err, test.expectedError)) }) } } diff --git a/fetcher/fetcher.go b/fetcher/fetcher.go index c08354ac..b0b4abc8 100644 --- a/fetcher/fetcher.go +++ b/fetcher/fetcher.go @@ -54,6 +54,12 @@ const ( DefaultUserAgent = "rosetta-sdk-go" ) +var ( + // ErrExhaustedRetries is returned when a fetch with retries + // fails because it was attempted too many times. + ErrExhaustedRetries = errors.New("retries exhausted") +) + // Fetcher contains all logic to communicate with a Rosetta Server. type Fetcher struct { // Asserter is a public variable because diff --git a/fetcher/utils.go b/fetcher/utils.go index bd7360e4..40518d6b 100644 --- a/fetcher/utils.go +++ b/fetcher/utils.go @@ -16,6 +16,7 @@ package fetcher import ( "log" + "strings" "time" "github.com/cenkalti/backoff" @@ -35,6 +36,7 @@ func backoffRetries( // tryAgain handles a backoff and prints error messages depending // on the fetchMsg. func tryAgain(fetchMsg string, thisBackoff backoff.BackOff, err error) bool { + fetchMsg = strings.Replace(fetchMsg, "\n", "", -1) log.Printf("%s fetch error: %s\n", fetchMsg, err.Error()) nextBackoff := thisBackoff.NextBackOff() From 5b5d5adad06d0abd6802e036166d205bd5cf884d Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 18:08:13 -0700 Subject: [PATCH 5/7] add fetcher tests for network --- fetcher/network.go | 23 ++- fetcher/network_test.go | 307 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 324 insertions(+), 6 deletions(-) create mode 100644 fetcher/network_test.go diff --git a/fetcher/network.go b/fetcher/network.go index 2ac4f9d9..66e4ce4f 100644 --- a/fetcher/network.go +++ b/fetcher/network.go @@ -16,7 +16,7 @@ package fetcher import ( "context" - "errors" + "fmt" "github.com/coinbase/rosetta-sdk-go/asserter" @@ -74,12 +74,16 @@ func (f *Fetcher) NetworkStatusRetry( return nil, ctx.Err() } - if !tryAgain("NetworkStatus", backoffRetries, err) { + if !tryAgain(fmt.Sprintf("network status %s", types.PrettyPrintStruct(network)), backoffRetries, err) { break } } - return nil, errors.New("exhausted retries for NetworkStatus") + return nil, fmt.Errorf( + "%w: unable to fetch network status %s", + ErrExhaustedRetries, + types.PrettyPrintStruct(network), + ) } // NetworkList returns the validated response @@ -134,7 +138,10 @@ func (f *Fetcher) NetworkListRetry( } } - return nil, errors.New("exhausted retries for NetworkList") + return nil, fmt.Errorf( + "%w: unable to fetch network list", + ErrExhaustedRetries, + ) } // NetworkOptions returns the validated response @@ -188,10 +195,14 @@ func (f *Fetcher) NetworkOptionsRetry( return nil, ctx.Err() } - if !tryAgain("NetworkOptions", backoffRetries, err) { + if !tryAgain(fmt.Sprintf("network options %s", types.PrettyPrintStruct(network)), backoffRetries, err) { break } } - return nil, errors.New("exhausted retries for NetworkOptions") + return nil, fmt.Errorf( + "%w: unable to fetch network options %s", + ErrExhaustedRetries, + types.PrettyPrintStruct(network), + ) } diff --git a/fetcher/network_test.go b/fetcher/network_test.go new file mode 100644 index 00000000..d27855fd --- /dev/null +++ b/fetcher/network_test.go @@ -0,0 +1,307 @@ +package fetcher + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/coinbase/rosetta-sdk-go/types" + + "github.com/stretchr/testify/assert" +) + +var ( + basicNetworkStatus = &types.NetworkStatusResponse{ + CurrentBlockIdentifier: basicBlock, + CurrentBlockTimestamp: 1582833600000, + GenesisBlockIdentifier: &types.BlockIdentifier{ + Index: 0, + Hash: "block 0", + }, + } + + basicNetworkList = &types.NetworkListResponse{ + NetworkIdentifiers: []*types.NetworkIdentifier{ + basicNetwork, + }, + } + + basicNetworkOptions = &types.NetworkOptionsResponse{ + Version: &types.Version{ + RosettaVersion: "1.3.1", + NodeVersion: "0.0.1", + }, + Allow: &types.Allow{ + OperationStatuses: []*types.OperationStatus{ + { + Status: "SUCCESS", + Successful: true, + }, + }, + OperationTypes: []string{"transfer"}, + }, + } +) + +func TestNetworkStatusRetry(t *testing.T) { + var tests = map[string]struct { + network *types.NetworkIdentifier + + errorsBeforeSuccess int + expectedStatus *types.NetworkStatusResponse + expectedError error + + fetcherMaxRetries uint64 + shouldCancel bool + }{ + "no failures": { + network: basicNetwork, + expectedStatus: basicNetworkStatus, + fetcherMaxRetries: 5, + }, + "retry failures": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedStatus: basicNetworkStatus, + fetcherMaxRetries: 5, + }, + "exhausted retries": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedError: ErrExhaustedRetries, + fetcherMaxRetries: 1, + }, + "cancel context": { + network: basicNetwork, + errorsBeforeSuccess: 6, + expectedError: context.Canceled, + fetcherMaxRetries: 5, + shouldCancel: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var ( + tries = 0 + assert = assert.New(t) + ctx, cancel = context.WithCancel(context.Background()) + endpoint = "/network/status" + ) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal("POST", r.Method) + assert.Equal(endpoint, r.URL.RequestURI()) + + expected := &types.NetworkRequest{ + NetworkIdentifier: test.network, + } + var networkRequest *types.NetworkRequest + assert.NoError(json.NewDecoder(r.Body).Decode(&networkRequest)) + assert.Equal(expected, networkRequest) + + if test.shouldCancel { + cancel() + } + + if tries < test.errorsBeforeSuccess { + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "{}") + tries++ + return + } + + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, types.PrettyPrintStruct(test.expectedStatus)) + })) + + defer ts.Close() + + f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + status, err := f.NetworkStatusRetry( + ctx, + test.network, + nil, + ) + assert.Equal(test.expectedStatus, status) + assert.True(errors.Is(err, test.expectedError)) + }) + } +} + +func TestNetworkListRetry(t *testing.T) { + var tests = map[string]struct { + network *types.NetworkIdentifier + + errorsBeforeSuccess int + expectedList *types.NetworkListResponse + expectedError error + + fetcherMaxRetries uint64 + shouldCancel bool + }{ + "no failures": { + network: basicNetwork, + expectedList: basicNetworkList, + fetcherMaxRetries: 5, + }, + "retry failures": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedList: basicNetworkList, + fetcherMaxRetries: 5, + }, + "exhausted retries": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedError: ErrExhaustedRetries, + fetcherMaxRetries: 1, + }, + "cancel context": { + network: basicNetwork, + errorsBeforeSuccess: 6, + expectedError: context.Canceled, + fetcherMaxRetries: 5, + shouldCancel: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var ( + tries = 0 + assert = assert.New(t) + ctx, cancel = context.WithCancel(context.Background()) + endpoint = "/network/list" + ) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal("POST", r.Method) + assert.Equal(endpoint, r.URL.RequestURI()) + + expected := &types.MetadataRequest{} + var metadataRequest *types.MetadataRequest + assert.NoError(json.NewDecoder(r.Body).Decode(&metadataRequest)) + assert.Equal(expected, metadataRequest) + + if test.shouldCancel { + cancel() + } + + if tries < test.errorsBeforeSuccess { + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "{}") + tries++ + return + } + + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, types.PrettyPrintStruct(test.expectedList)) + })) + + defer ts.Close() + + f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + list, err := f.NetworkListRetry( + ctx, + nil, + ) + assert.Equal(test.expectedList, list) + assert.True(errors.Is(err, test.expectedError)) + }) + } +} + +func TestNetworkOptionsRetry(t *testing.T) { + var tests = map[string]struct { + network *types.NetworkIdentifier + + errorsBeforeSuccess int + expectedOptions *types.NetworkOptionsResponse + expectedError error + + fetcherMaxRetries uint64 + shouldCancel bool + }{ + "no failures": { + network: basicNetwork, + expectedOptions: basicNetworkOptions, + fetcherMaxRetries: 5, + }, + "retry failures": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedOptions: basicNetworkOptions, + fetcherMaxRetries: 5, + }, + "exhausted retries": { + network: basicNetwork, + errorsBeforeSuccess: 2, + expectedError: ErrExhaustedRetries, + fetcherMaxRetries: 1, + }, + "cancel context": { + network: basicNetwork, + errorsBeforeSuccess: 6, + expectedError: context.Canceled, + fetcherMaxRetries: 5, + shouldCancel: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var ( + tries = 0 + assert = assert.New(t) + ctx, cancel = context.WithCancel(context.Background()) + endpoint = "/network/options" + ) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal("POST", r.Method) + assert.Equal(endpoint, r.URL.RequestURI()) + + expected := &types.NetworkRequest{ + NetworkIdentifier: test.network, + } + var networkRequest *types.NetworkRequest + assert.NoError(json.NewDecoder(r.Body).Decode(&networkRequest)) + assert.Equal(expected, networkRequest) + + if test.shouldCancel { + cancel() + } + + if tries < test.errorsBeforeSuccess { + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "{}") + tries++ + return + } + + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, types.PrettyPrintStruct(test.expectedOptions)) + })) + + defer ts.Close() + + f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + options, err := f.NetworkOptionsRetry( + ctx, + test.network, + nil, + ) + assert.Equal(test.expectedOptions, options) + assert.True(errors.Is(err, test.expectedError)) + }) + } +} From 83518850ab74ba598ccc1fee85da0e418f0f5ff2 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 18:37:55 -0700 Subject: [PATCH 6/7] add tests for fetcher block --- fetcher/block.go | 7 ++- fetcher/block_test.go | 141 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 fetcher/block_test.go diff --git a/fetcher/block.go b/fetcher/block.go index 6aa6e070..35bf9ca9 100644 --- a/fetcher/block.go +++ b/fetcher/block.go @@ -16,7 +16,6 @@ package fetcher import ( "context" - "errors" "fmt" "github.com/coinbase/rosetta-sdk-go/asserter" @@ -227,7 +226,11 @@ func (f *Fetcher) BlockRetry( } } - return nil, errors.New("exhausted retries for block") + return nil, fmt.Errorf( + "%w: unable to fetch block %s", + ErrExhaustedRetries, + types.PrettyPrintStruct(blockIdentifier), + ) } // addIndicies appends a range of indicies (from diff --git a/fetcher/block_test.go b/fetcher/block_test.go new file mode 100644 index 00000000..fc3663bc --- /dev/null +++ b/fetcher/block_test.go @@ -0,0 +1,141 @@ +package fetcher + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/coinbase/rosetta-sdk-go/asserter" + "github.com/coinbase/rosetta-sdk-go/types" + + "github.com/stretchr/testify/assert" +) + +var ( + basicFullBlock = &types.Block{ + BlockIdentifier: basicBlock, + ParentBlockIdentifier: &types.BlockIdentifier{ + Index: 9, + Hash: "block 9", + }, + Timestamp: 1582833600000, + } +) + +func TestBlockRetry(t *testing.T) { + var tests = map[string]struct { + network *types.NetworkIdentifier + block *types.BlockIdentifier + + errorsBeforeSuccess int + expectedBlock *types.Block + expectedError error + + fetcherMaxRetries uint64 + shouldCancel bool + }{ + "no failures": { + network: basicNetwork, + block: basicBlock, + expectedBlock: basicFullBlock, + fetcherMaxRetries: 5, + }, + "retry failures": { + network: basicNetwork, + block: basicBlock, + errorsBeforeSuccess: 2, + expectedBlock: basicFullBlock, + fetcherMaxRetries: 5, + }, + "exhausted retries": { + network: basicNetwork, + block: basicBlock, + errorsBeforeSuccess: 2, + expectedError: ErrExhaustedRetries, + fetcherMaxRetries: 1, + }, + "cancel context": { + network: basicNetwork, + block: basicBlock, + errorsBeforeSuccess: 6, + expectedError: context.Canceled, + fetcherMaxRetries: 5, + shouldCancel: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var ( + tries = 0 + assert = assert.New(t) + ctx, cancel = context.WithCancel(context.Background()) + endpoint = "/block" + ) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal("POST", r.Method) + assert.Equal(endpoint, r.URL.RequestURI()) + + expected := &types.BlockRequest{ + NetworkIdentifier: test.network, + BlockIdentifier: types.ConstructPartialBlockIdentifier(test.block), + } + var blockRequest *types.BlockRequest + assert.NoError(json.NewDecoder(r.Body).Decode(&blockRequest)) + assert.Equal(expected, blockRequest) + + if test.shouldCancel { + cancel() + } + + if tries < test.errorsBeforeSuccess { + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintln(w, "{}") + tries++ + return + } + + w.Header().Set("Content-Type", "application/json; charset=UTF-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, types.PrettyPrintStruct( + &types.BlockResponse{ + Block: test.expectedBlock, + }, + )) + })) + + defer ts.Close() + a, err := asserter.NewClientWithOptions( + basicNetwork, + &types.BlockIdentifier{ + Index: 0, + Hash: "block 0", + }, + basicNetworkOptions.Allow.OperationTypes, + basicNetworkOptions.Allow.OperationStatuses, + nil, + ) + assert.NoError(err) + + f := New( + ts.URL, + WithRetryElapsedTime(5*time.Second), + WithMaxRetries(test.fetcherMaxRetries), + WithAsserter(a), + ) + block, err := f.BlockRetry( + ctx, + test.network, + types.ConstructPartialBlockIdentifier(test.block), + ) + assert.Equal(test.expectedBlock, block) + assert.True(errors.Is(err, test.expectedError)) + }) + } +} From 59c4ca849c591cf130f0fb2fb784b171190cb046 Mon Sep 17 00:00:00 2001 From: Patrick O'Grady Date: Mon, 18 May 2020 18:40:51 -0700 Subject: [PATCH 7/7] nits --- fetcher/account.go | 6 +++++- fetcher/account_test.go | 24 +++++++++++++++++++----- fetcher/block_test.go | 14 ++++++++++++++ fetcher/network.go | 12 ++++++++++-- fetcher/network_test.go | 32 +++++++++++++++++++++++++++++--- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/fetcher/account.go b/fetcher/account.go index 97921f52..d7a7470b 100644 --- a/fetcher/account.go +++ b/fetcher/account.go @@ -85,7 +85,11 @@ func (f *Fetcher) AccountBalanceRetry( return nil, nil, nil, ctx.Err() } - if !tryAgain(fmt.Sprintf("account %s", types.PrettyPrintStruct(account)), backoffRetries, err) { + if !tryAgain( + fmt.Sprintf("account %s", types.PrettyPrintStruct(account)), + backoffRetries, + err, + ) { break } } diff --git a/fetcher/account_test.go b/fetcher/account_test.go index f988a21a..e3b97402 100644 --- a/fetcher/account_test.go +++ b/fetcher/account_test.go @@ -1,3 +1,17 @@ +// Copyright 2020 Coinbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package fetcher import ( @@ -30,10 +44,6 @@ var ( Hash: "block 10", } - basicPartialBlock = &types.PartialBlockIdentifier{ - Index: &basicBlock.Index, - } - basicAmounts = []*types.Amount{ { Value: "1000", @@ -134,7 +144,11 @@ func TestAccountBalanceRetry(t *testing.T) { defer ts.Close() - f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + f := New( + ts.URL, + WithRetryElapsedTime(5*time.Second), + WithMaxRetries(test.fetcherMaxRetries), + ) block, amounts, metadata, err := f.AccountBalanceRetry( ctx, test.network, diff --git a/fetcher/block_test.go b/fetcher/block_test.go index fc3663bc..96ea4b67 100644 --- a/fetcher/block_test.go +++ b/fetcher/block_test.go @@ -1,3 +1,17 @@ +// Copyright 2020 Coinbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package fetcher import ( diff --git a/fetcher/network.go b/fetcher/network.go index 66e4ce4f..1a26da31 100644 --- a/fetcher/network.go +++ b/fetcher/network.go @@ -74,7 +74,11 @@ func (f *Fetcher) NetworkStatusRetry( return nil, ctx.Err() } - if !tryAgain(fmt.Sprintf("network status %s", types.PrettyPrintStruct(network)), backoffRetries, err) { + if !tryAgain( + fmt.Sprintf("network status %s", types.PrettyPrintStruct(network)), + backoffRetries, + err, + ) { break } } @@ -195,7 +199,11 @@ func (f *Fetcher) NetworkOptionsRetry( return nil, ctx.Err() } - if !tryAgain(fmt.Sprintf("network options %s", types.PrettyPrintStruct(network)), backoffRetries, err) { + if !tryAgain( + fmt.Sprintf("network options %s", types.PrettyPrintStruct(network)), + backoffRetries, + err, + ) { break } } diff --git a/fetcher/network_test.go b/fetcher/network_test.go index d27855fd..1b17ad97 100644 --- a/fetcher/network_test.go +++ b/fetcher/network_test.go @@ -1,3 +1,17 @@ +// Copyright 2020 Coinbase, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package fetcher import ( @@ -123,7 +137,11 @@ func TestNetworkStatusRetry(t *testing.T) { defer ts.Close() - f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + f := New( + ts.URL, + WithRetryElapsedTime(5*time.Second), + WithMaxRetries(test.fetcherMaxRetries), + ) status, err := f.NetworkStatusRetry( ctx, test.network, @@ -208,7 +226,11 @@ func TestNetworkListRetry(t *testing.T) { defer ts.Close() - f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + f := New( + ts.URL, + WithRetryElapsedTime(5*time.Second), + WithMaxRetries(test.fetcherMaxRetries), + ) list, err := f.NetworkListRetry( ctx, nil, @@ -294,7 +316,11 @@ func TestNetworkOptionsRetry(t *testing.T) { defer ts.Close() - f := New(ts.URL, WithRetryElapsedTime(5*time.Second), WithMaxRetries(test.fetcherMaxRetries)) + f := New( + ts.URL, + WithRetryElapsedTime(5*time.Second), + WithMaxRetries(test.fetcherMaxRetries), + ) options, err := f.NetworkOptionsRetry( ctx, test.network,