From 39a42500996abce92728eeaeda5eb3e469d61b3f Mon Sep 17 00:00:00 2001 From: Divyank Katira Date: Tue, 13 Feb 2018 17:35:07 -0500 Subject: [PATCH] [FAB-8261] Introduce Multi Errors type This type is used to represent multiple errors, typically from multiple server targets invoked by the SDK in the same operation. Change-Id: I718478751c93e5b430b929dfd82f1d263bf864f7 Signed-off-by: Divyank Katira --- api/apifabclient/mocks/mockfabclient.gen.go | 4 +- api/apifabclient/proposer.go | 12 +- api/apitxn/resmgmtclient/resmgmt.go | 1 - pkg/errors/multi/multi.go | 72 +++++ pkg/errors/multi/multi_test.go | 63 +++++ pkg/errors/status/codes.go | 4 + pkg/errors/status/status.go | 8 +- pkg/errors/status/status_test.go | 9 + pkg/fabric-client/channel/channel.go | 4 +- pkg/fabric-client/channel/ledger.go | 263 ++++++------------- pkg/fabric-client/channel/ledger_test.go | 20 ++ pkg/fabric-client/channel/txn_test.go | 4 +- pkg/fabric-client/mocks/mockpeer.go | 6 +- pkg/fabric-client/mocks/mockresource.go | 7 +- pkg/fabric-client/peer/deprecated_test.go | 2 +- pkg/fabric-client/peer/peer.go | 2 +- pkg/fabric-client/peer/peer_test.go | 2 +- pkg/fabric-client/peer/peerendorser.go | 6 +- pkg/fabric-client/peer/peerendorser_test.go | 2 +- pkg/fabric-client/resource/resource.go | 31 +-- pkg/fabric-client/txn/proposal.go | 19 +- pkg/fabric-client/txn/proposal_test.go | 16 +- pkg/fabric-client/txn/txn_test.go | 52 ++-- pkg/fabric-txn/chclient/chclient.go | 28 +- pkg/fabric-txn/chclient/chclient_test.go | 35 ++- pkg/fabric-txn/resmgmtclient/resmgmt.go | 28 +- pkg/fabric-txn/resmgmtclient/resmgmt_test.go | 10 +- pkg/fabric-txn/txnhandler/txnhandler.go | 15 +- test/integration/base_test_setup.go | 14 +- 29 files changed, 374 insertions(+), 365 deletions(-) create mode 100644 pkg/errors/multi/multi.go create mode 100644 pkg/errors/multi/multi_test.go diff --git a/api/apifabclient/mocks/mockfabclient.gen.go b/api/apifabclient/mocks/mockfabclient.gen.go index d523f4add2..6eb1c286bc 100644 --- a/api/apifabclient/mocks/mockfabclient.gen.go +++ b/api/apifabclient/mocks/mockfabclient.gen.go @@ -35,9 +35,9 @@ func (m *MockProposalProcessor) EXPECT() *MockProposalProcessorMockRecorder { } // ProcessTransactionProposal mocks base method -func (m *MockProposalProcessor) ProcessTransactionProposal(arg0 apifabclient.TransactionProposal) (apifabclient.TransactionProposalResult, error) { +func (m *MockProposalProcessor) ProcessTransactionProposal(arg0 apifabclient.TransactionProposal) (apifabclient.TransactionProposalResponse, error) { ret := m.ctrl.Call(m, "ProcessTransactionProposal", arg0) - ret0, _ := ret[0].(apifabclient.TransactionProposalResult) + ret0, _ := ret[0].(apifabclient.TransactionProposalResponse) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/api/apifabclient/proposer.go b/api/apifabclient/proposer.go index b8cc26a109..99ab01cf89 100644 --- a/api/apifabclient/proposer.go +++ b/api/apifabclient/proposer.go @@ -12,7 +12,7 @@ import ( // ProposalProcessor simulates transaction proposal, so that a client can submit the result for ordering. type ProposalProcessor interface { - ProcessTransactionProposal(proposal TransactionProposal) (TransactionProposalResult, error) + ProcessTransactionProposal(proposal TransactionProposal) (TransactionProposalResponse, error) } // ProposalSender provides the ability for a transaction proposal to be created and sent. @@ -43,19 +43,11 @@ type TransactionProposal struct { Proposal *pb.Proposal } -// TransactionProposalResponse encapsulates both the result of transaction proposal processing and errors. +// TransactionProposalResponse respresents the result of transaction proposal processing. type TransactionProposalResponse struct { - TransactionProposalResult - Err error // TODO: consider refactoring -} - -// TransactionProposalResult respresents the result of transaction proposal processing. -type TransactionProposalResult struct { Endorser string Status int32 Proposal TransactionProposal ProposalResponse *pb.ProposalResponse } - -// TODO: TransactionProposalResponse and TransactionProposalResult may need better names. diff --git a/api/apitxn/resmgmtclient/resmgmt.go b/api/apitxn/resmgmtclient/resmgmt.go index 425fc7ecb7..b23a365c5c 100644 --- a/api/apitxn/resmgmtclient/resmgmt.go +++ b/api/apitxn/resmgmtclient/resmgmt.go @@ -32,7 +32,6 @@ type InstallCCResponse struct { Target string Status int32 Info string - Err error } // InstantiateCCRequest contains instantiate chaincode request parameters diff --git a/pkg/errors/multi/multi.go b/pkg/errors/multi/multi.go new file mode 100644 index 0000000000..c5b412e542 --- /dev/null +++ b/pkg/errors/multi/multi.go @@ -0,0 +1,72 @@ +/* +Copyright SecureKey Technologies Inc. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package multi + +import ( + "fmt" + "strings" +) + +// Errors is used to represent multiple errors +type Errors []error + +// New Errors object with the given errors. Only non-nil errors are added. +func New(errs ...error) error { + errors := Errors{} + for _, err := range errs { + if err != nil { + errors = append(errors, err) + } + } + + if len(errors) == 0 { + return nil + } + + if len(errors) == 1 { + return errors[0] + } + + return errors +} + +// Append error to Errors. If the first arg is not an Errors object, one will be created +func Append(errs error, err error) error { + m, ok := errs.(Errors) + if !ok { + return New(errs, err) + } + return append(m, err) +} + +// ToError converts Errors to the error interface +// returns nil if no errors are present, a single error object if only one is present +func (errs Errors) ToError() error { + if len(errs) == 0 { + return nil + } + if len(errs) == 1 { + return errs[0] + } + return errs +} + +// Error implements the error interface to return a string representation of Errors +func (errs Errors) Error() string { + if len(errs) == 0 { + return "" + } + if len(errs) == 1 { + return errs[0].Error() + } + + errors := []string{fmt.Sprint("Multiple errors occurred: ")} + for _, err := range errs { + errors = append(errors, err.Error()) + } + return strings.Join(errors, "\n") +} diff --git a/pkg/errors/multi/multi_test.go b/pkg/errors/multi/multi_test.go new file mode 100644 index 0000000000..d369f7b244 --- /dev/null +++ b/pkg/errors/multi/multi_test.go @@ -0,0 +1,63 @@ +/* +Copyright SecureKey Technologies Inc. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package multi + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestErrorString(t *testing.T) { + testErr := fmt.Errorf("test") + var errs Errors + + assert.Equal(t, "", errs.Error()) + + errs = append(errs, testErr) + assert.Equal(t, testErr.Error(), errs.Error()) + + errs = append(errs, testErr) + assert.Equal(t, "Multiple errors occurred: \ntest\ntest", errs.Error()) +} + +func TestAppend(t *testing.T) { + testErr := fmt.Errorf("test") + testErr2 := fmt.Errorf("test2") + + m := Append(nil, nil) + assert.Nil(t, m) + + m = Append(nil, testErr) + assert.Equal(t, testErr, m) + + m = Append(testErr, testErr2) + m1, ok := m.(Errors) + assert.True(t, ok) + assert.Equal(t, testErr, m1[0]) + assert.Equal(t, testErr2, m1[1]) + + m = Append(Errors{testErr}, testErr2) + m1, ok = m.(Errors) + assert.True(t, ok) + assert.Equal(t, testErr, m1[0]) + assert.Equal(t, testErr2, m1[1]) +} + +func TestToError(t *testing.T) { + testErr := fmt.Errorf("test") + var errs Errors + + assert.Equal(t, nil, errs.ToError()) + + errs = append(errs, testErr) + assert.Equal(t, testErr, errs.ToError()) + + errs = append(errs, testErr) + assert.Equal(t, errs, errs.ToError()) +} diff --git a/pkg/errors/status/codes.go b/pkg/errors/status/codes.go index 5d62ad90f1..0f789e6e28 100644 --- a/pkg/errors/status/codes.go +++ b/pkg/errors/status/codes.go @@ -38,6 +38,9 @@ const ( // NoPeersFound No peers were discovered/configured NoPeersFound Code = 6 + + // MultipleErrors multiple errors occurred + MultipleErrors Code = 7 ) // CodeName maps the codes in this packages to human-readable strings @@ -49,6 +52,7 @@ var CodeName = map[int32]string{ 4: "EMPTY_CERT", 5: "TIMEOUT", 6: "NO_PEERS_FOUND", + 7: "MULTIPLE_ERRORS", } // ToInt32 cast to int32 diff --git a/pkg/errors/status/status.go b/pkg/errors/status/status.go index 76b21e7811..c0e31a68cd 100644 --- a/pkg/errors/status/status.go +++ b/pkg/errors/status/status.go @@ -14,6 +14,7 @@ import ( "github.com/pkg/errors" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" grpcstatus "google.golang.org/grpc/status" ) @@ -94,8 +95,8 @@ func (g Group) String() string { return UnknownStatus.String() } -// FromError returns a Status representing err if it was produced from this -// package, otherwise it returns nil, false. +// FromError returns a Status representing err if available, +// otherwise it returns nil, false. func FromError(err error) (s *Status, ok bool) { if err == nil { return &Status{Code: int32(OK)}, true @@ -107,6 +108,9 @@ func FromError(err error) (s *Status, ok bool) { if s, ok := unwrappedErr.(*Status); ok { return s, true } + if m, ok := unwrappedErr.(multi.Errors); ok { + return New(ClientStatus, MultipleErrors.ToInt32(), m.Error(), nil), true + } return nil, false } diff --git a/pkg/errors/status/status_test.go b/pkg/errors/status/status_test.go index 93a1d9a753..8da56feac7 100644 --- a/pkg/errors/status/status_test.go +++ b/pkg/errors/status/status_test.go @@ -10,6 +10,7 @@ import ( "fmt" "testing" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/common" pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" "github.com/pkg/errors" @@ -65,6 +66,14 @@ func TestFromError(t *testing.T) { s, ok = FromError(fmt.Errorf("Test")) assert.False(t, ok) + + errs := multi.Errors{} + errs = append(errs, fmt.Errorf("Test")) + s, ok = FromError(errs) + assert.True(t, ok) + assert.Equal(t, ClientStatus, s.Group) + assert.EqualValues(t, MultipleErrors.ToInt32(), s.Code) + assert.Equal(t, errs.Error(), s.Message) } func TestStatusToError(t *testing.T) { diff --git a/pkg/fabric-client/channel/channel.go b/pkg/fabric-client/channel/channel.go index 4771cea923..7e2d6b2a04 100644 --- a/pkg/fabric-client/channel/channel.go +++ b/pkg/fabric-client/channel/channel.go @@ -431,7 +431,7 @@ func (c *Channel) QueryByChaincode(request fab.ChaincodeInvokeRequest) ([][]byte return nil, err } resps, err := queryChaincode(c.clientContext, c.name, request, targets) - return filterProposalResponses(resps, err) + return collectProposalResponses(resps), err } // QueryBySystemChaincode invokes a chaincode that isn't part of a channel. @@ -443,5 +443,5 @@ func (c *Channel) QueryBySystemChaincode(request fab.ChaincodeInvokeRequest) ([] return nil, err } resps, err := queryChaincode(c.clientContext, systemChannel, request, targets) - return filterProposalResponses(resps, err) + return collectProposalResponses(resps), err } diff --git a/pkg/fabric-client/channel/ledger.go b/pkg/fabric-client/channel/ledger.go index 22758146d6..3e194ccfe2 100644 --- a/pkg/fabric-client/channel/ledger.go +++ b/pkg/fabric-client/channel/ledger.go @@ -10,12 +10,12 @@ import ( "bytes" "net/http" "strconv" - "strings" "github.com/golang/protobuf/proto" "github.com/pkg/errors" fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/txn" "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/common" pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" @@ -54,32 +54,31 @@ func (c *Ledger) QueryInfo(targets []fab.ProposalProcessor) ([]*common.Blockchai Fcn: "GetChainInfo", Args: args, } - tprs, err := queryChaincode(c.ctx, systemChannel, request, targets) - processed, err := processTxnProposalResponse(tprs, err, createBlockchainInfo) + tprs, errs := queryChaincode(c.ctx, systemChannel, request, targets) responses := []*common.BlockchainInfo{} - for _, p := range processed { - responses = append(responses, p.(*common.BlockchainInfo)) + for _, tpr := range tprs { + r, err := createBlockchainInfo(tpr) + if err != nil { + errs = multi.Append(errs, errors.WithMessage(err, "From target: "+tpr.Endorser)) + } else { + responses = append(responses, r) + } } - return responses, err + return responses, errs } -func createBlockchainInfo(tpr *fab.TransactionProposalResponse, err error) (interface{}, error) { +func createBlockchainInfo(tpr *fab.TransactionProposalResponse) (*common.BlockchainInfo, error) { response := common.BlockchainInfo{} - if err != nil { - // response had an error - do not process. - return &response, err - } - - err = proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) + err := proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) if err != nil { return nil, errors.Wrap(err, "unmarshal of transaction proposal response failed") } - return &response, err + return &response, nil } // QueryBlockByHash queries the ledger for Block by block hash. -// This query will be made to the primary peer. +// This query will be made to specified targets. // Returns the block. func (c *Ledger) QueryBlockByHash(blockHash []byte, targets []fab.ProposalProcessor) ([]*common.Block, error) { @@ -97,18 +96,22 @@ func (c *Ledger) QueryBlockByHash(blockHash []byte, targets []fab.ProposalProces Fcn: "GetBlockByHash", Args: args, } - tprs, err := queryChaincode(c.ctx, systemChannel, request, targets) - processed, err := processTxnProposalResponse(tprs, err, createCommonBlock) + tprs, errs := queryChaincode(c.ctx, systemChannel, request, targets) responses := []*common.Block{} - for _, p := range processed { - responses = append(responses, p.(*common.Block)) + for _, tpr := range tprs { + r, err := createCommonBlock(tpr) + if err != nil { + errs = multi.Append(errs, errors.WithMessage(err, "From target: "+tpr.Endorser)) + } else { + responses = append(responses, r) + } } - return responses, err + return responses, errs } // QueryBlock queries the ledger for Block by block number. -// This query will be made to the primary peer. +// This query will be made to specified targets. // blockNumber: The number which is the ID of the Block. // It returns the block. func (c *Ledger) QueryBlock(blockNumber int, targets []fab.ProposalProcessor) ([]*common.Block, error) { @@ -128,24 +131,23 @@ func (c *Ledger) QueryBlock(blockNumber int, targets []fab.ProposalProcessor) ([ Args: args, } - tprs, err := queryChaincode(c.ctx, systemChannel, request, targets) - processed, err := processTxnProposalResponse(tprs, err, createCommonBlock) + tprs, errs := queryChaincode(c.ctx, systemChannel, request, targets) responses := []*common.Block{} - for _, p := range processed { - responses = append(responses, p.(*common.Block)) + for _, tpr := range tprs { + r, err := createCommonBlock(tpr) + if err != nil { + errs = multi.Append(errs, errors.WithMessage(err, "From target: "+tpr.Endorser)) + } else { + responses = append(responses, r) + } } - return responses, err + return responses, errs } -func createCommonBlock(tpr *fab.TransactionProposalResponse, err error) (interface{}, error) { +func createCommonBlock(tpr *fab.TransactionProposalResponse) (*common.Block, error) { response := common.Block{} - if err != nil { - // response had an error - do not process. - return &response, err - } - - err = proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) + err := proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) if err != nil { return nil, errors.Wrap(err, "unmarshal of transaction proposal response failed") } @@ -153,9 +155,8 @@ func createCommonBlock(tpr *fab.TransactionProposalResponse, err error) (interfa } // QueryTransaction queries the ledger for Transaction by number. -// This query will be made to the primary peer. +// This query will be made to specified targets. // Returns the ProcessedTransaction information containing the transaction. -// TODO: add optional target func (c *Ledger) QueryTransaction(transactionID string, targets []fab.ProposalProcessor) ([]*pb.ProcessedTransaction, error) { // prepare arguments to call qscc GetTransactionByID function @@ -169,24 +170,24 @@ func (c *Ledger) QueryTransaction(transactionID string, targets []fab.ProposalPr Args: args, } - tprs, err := queryChaincode(c.ctx, systemChannel, request, targets) - processed, err := processTxnProposalResponse(tprs, err, createProcessedTransaction) + tprs, errs := queryChaincode(c.ctx, systemChannel, request, targets) responses := []*pb.ProcessedTransaction{} - for _, p := range processed { - responses = append(responses, p.(*pb.ProcessedTransaction)) + for _, tpr := range tprs { + r, err := createProcessedTransaction(tpr) + if err != nil { + errs = multi.Append(errs, errors.WithMessage(err, "From target: "+tpr.Endorser)) + } else { + responses = append(responses, r) + } } - return responses, err + + return responses, errs } -func createProcessedTransaction(tpr *fab.TransactionProposalResponse, err error) (interface{}, error) { +func createProcessedTransaction(tpr *fab.TransactionProposalResponse) (*pb.ProcessedTransaction, error) { response := pb.ProcessedTransaction{} - if err != nil { - // response had an error - do not process. - return &response, err - } - - err = proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) + err := proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) if err != nil { return nil, errors.Wrap(err, "unmarshal of transaction proposal response failed") } @@ -194,35 +195,34 @@ func createProcessedTransaction(tpr *fab.TransactionProposalResponse, err error) } // QueryInstantiatedChaincodes queries the instantiated chaincodes on this channel. -// This query will be made to the primary peer. +// This query will be made to specified targets. func (c *Ledger) QueryInstantiatedChaincodes(targets []fab.ProposalProcessor) ([]*pb.ChaincodeQueryResponse, error) { request := fab.ChaincodeInvokeRequest{ ChaincodeID: "lscc", Fcn: "getchaincodes", } - tprs, err := queryChaincode(c.ctx, c.chName, request, targets) - processed, err := processTxnProposalResponse(tprs, err, createChaincodeQueryResponse) + tprs, errs := queryChaincode(c.ctx, c.chName, request, targets) responses := []*pb.ChaincodeQueryResponse{} - for _, p := range processed { - responses = append(responses, p.(*pb.ChaincodeQueryResponse)) + for _, tpr := range tprs { + r, err := createChaincodeQueryResponse(tpr) + if err != nil { + errs = multi.Append(errs, errors.WithMessage(err, "From target: "+tpr.Endorser)) + } else { + responses = append(responses, r) + } } - return responses, err + return responses, errs } -func createChaincodeQueryResponse(tpr *fab.TransactionProposalResponse, err error) (interface{}, error) { +func createChaincodeQueryResponse(tpr *fab.TransactionProposalResponse) (*pb.ChaincodeQueryResponse, error) { response := pb.ChaincodeQueryResponse{} - if err != nil { - // response had an error - do not process. - return &response, err - } - - err = proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) + err := proto.Unmarshal(tpr.ProposalResponse.GetResponse().Payload, &response) if err != nil { return nil, errors.Wrap(err, "unmarshal of transaction proposal response failed") } - return &response, err + return &response, nil } // QueryConfigBlock returns the current configuration block for the specified channel. If the @@ -243,14 +243,11 @@ func (c *Ledger) QueryConfigBlock(peers []fab.Peer, minResponses int) (*common.C Args: [][]byte{[]byte(c.chName)}, } tpr, err := queryChaincode(c.ctx, c.chName, request, peersToTxnProcessors(peers)) - if err != nil { + if err != nil && len(tpr) == 0 { return nil, errors.WithMessage(err, "queryChaincode failed") } - responses, err := filterProposalResponses(tpr, err) - if err != nil { - return nil, err - } + responses := collectProposalResponses(tpr) if len(responses) < minResponses { return nil, errors.Errorf("Required minimum %d endorsments got %d", minResponses, len(responses)) @@ -281,138 +278,34 @@ func (c *Ledger) QueryConfigBlock(peers []fab.Peer, minResponses int) (*common.C } -type txnProposalResponseOp func(*fab.TransactionProposalResponse, error) (interface{}, error) - -func processTxnProposalResponse(tprs []*fab.TransactionProposalResponse, tperr error, op txnProposalResponseOp) ([]interface{}, error) { - - // examine errors from peers and prepare error slice that can be checked during each response' processing. - var errs MultiError - if tperr != nil { - var ok bool - errs, ok = tperr.(MultiError) - if !ok { - return nil, errors.WithMessage(tperr, "chaincode query failed") - } - } else { - errs = make([]error, len(tprs)) - } - - // process each response and set processing error, if needed. - responses := []interface{}{} - var resperrs MultiError - isErr := false - for i, tpr := range tprs { - var resp interface{} - var err error - - resp, err = op(tpr, errs[i]) - if err != nil { - isErr = true - } - - responses = append(responses, resp) - resperrs = append(resperrs, err) - } - - // when any error has occurred return responses and errors as a MultiError. - if isErr { - return responses, resperrs - } - return responses, nil -} - -func filterProposalResponses(tprs []*fab.TransactionProposalResponse, tperr error) ([][]byte, error) { - // examine errors from peers and prepare error slice that can be checked during each response' processing. - var errs MultiError - if tperr != nil { - var ok bool - errs, ok = tperr.(MultiError) - if !ok { - return nil, errors.WithMessage(tperr, "chaincode query failed") - } - } else { - errs = make(MultiError, len(tprs)) - } - +func collectProposalResponses(tprs []*fab.TransactionProposalResponse) [][]byte { responses := [][]byte{} - errMsg := "" - for i, tpr := range tprs { - if errs[i] != nil { - errMsg = errMsg + errs[i].Error() + "\n" - } else { - responses = append(responses, tpr.ProposalResponse.GetResponse().Payload) - } - } - - if len(errMsg) > 0 { - return responses, errors.New(errMsg) + for _, tpr := range tprs { + responses = append(responses, tpr.ProposalResponse.GetResponse().Payload) } - return responses, nil -} - -// MultiError represents a slice of errors originating from each target peer. -type MultiError []error -func (me MultiError) Error() string { - msg := []string{} - for _, e := range me { - msg = append(msg, e.Error()) - } - return strings.Join(msg, ",") + return responses } func queryChaincode(ctx fab.Context, channel string, request fab.ChaincodeInvokeRequest, targets []fab.ProposalProcessor) ([]*fab.TransactionProposalResponse, error) { - errors := MultiError{} - responses := []*fab.TransactionProposalResponse{} - isErr := false - - // TODO: this can be done concurrently. - for _, target := range targets { - resp, err := queryChaincodeWithTarget(ctx, channel, request, target) - - responses = append(responses, resp) - errors = append(errors, err) - - if err != nil { - isErr = true - } - } - if isErr { - return responses, errors - } - return responses, nil -} - -func queryChaincodeWithTarget(ctx fab.Context, channel string, request fab.ChaincodeInvokeRequest, target fab.ProposalProcessor) (*fab.TransactionProposalResponse, error) { - - targets := []fab.ProposalProcessor{target} - tp, err := txn.NewProposal(ctx, channel, request) if err != nil { return nil, errors.WithMessage(err, "NewProposal failed") } + tprs, errs := txn.SendProposal(tp, targets) - tpr, err := txn.SendProposal(tp, targets) - if err != nil { - return nil, errors.WithMessage(err, "SendProposal failed") - } - - err = validateResponse(tpr[0]) - if err != nil { - return nil, errors.WithMessage(err, "transaction proposal failed") - } - - return tpr[0], nil + return filterResponses(tprs, errs) } -func validateResponse(response *fab.TransactionProposalResponse) error { - if response.Err != nil { - return errors.Errorf("error from %s (%s)", response.Endorser, response.Err.Error()) - } - - if response.Status != http.StatusOK { - return errors.Errorf("bad status from %s (%d)", response.Endorser, response.Status) +func filterResponses(responses []*fab.TransactionProposalResponse, errs error) ([]*fab.TransactionProposalResponse, error) { + filteredResponses := responses[:0] + for _, response := range responses { + if response.Status == http.StatusOK { + filteredResponses = append(filteredResponses, response) + } else { + errs = multi.Append(errs, errors.Errorf("bad status from %s (%d)", response.Endorser, response.Status)) + } } - return nil + return filteredResponses, errs } diff --git a/pkg/fabric-client/channel/ledger_test.go b/pkg/fabric-client/channel/ledger_test.go index b1b6b9d553..f8b691103c 100644 --- a/pkg/fabric-client/channel/ledger_test.go +++ b/pkg/fabric-client/channel/ledger_test.go @@ -6,11 +6,14 @@ SPDX-License-Identifier: Apache-2.0 package channel import ( + "fmt" "testing" "github.com/golang/protobuf/proto" fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/mocks" + "github.com/stretchr/testify/assert" ) func TestQueryMethods(t *testing.T) { @@ -174,6 +177,23 @@ func TestQueryConfig(t *testing.T) { } +func TestFilterResponses(t *testing.T) { + tprs := []*fab.TransactionProposalResponse{} + err := fmt.Errorf("test") + for i := 0; i <= 100; i++ { + var s int + if i%2 == 0 { + s = 200 + } else { + s = 500 + } + tprs = append(tprs, &fab.TransactionProposalResponse{Status: int32(s)}) + } + f, errs := filterResponses(tprs, err) + assert.Len(t, f, 51) + assert.Len(t, errs.(multi.Errors), 51) +} + func setupTestLedger() (*Ledger, error) { return setupLedger("testChannel") } diff --git a/pkg/fabric-client/channel/txn_test.go b/pkg/fabric-client/channel/txn_test.go index e98b7a66b6..74b915b6a2 100644 --- a/pkg/fabric-client/channel/txn_test.go +++ b/pkg/fabric-client/channel/txn_test.go @@ -46,7 +46,7 @@ func TestSendInstantiateProposal(t *testing.T) { proc := mock_fab.NewMockProposalProcessor(mockCtrl) tp := fab.TransactionProposal{SignedProposal: &pb.SignedProposal{}} - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(gomock.Any()).Return(tpr, nil) proc.EXPECT().ProcessTransactionProposal(gomock.Any()).Return(tpr, nil) @@ -122,7 +122,7 @@ func TestSendUpgradeProposal(t *testing.T) { proc := mock_fab.NewMockProposalProcessor(mockCtrl) tp := fab.TransactionProposal{SignedProposal: &pb.SignedProposal{}} - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(gomock.Any()).Return(tpr, nil) targets := []fab.ProposalProcessor{proc} diff --git a/pkg/fabric-client/mocks/mockpeer.go b/pkg/fabric-client/mocks/mockpeer.go index c30f1fa3d5..9d610ccf98 100644 --- a/pkg/fabric-client/mocks/mockpeer.go +++ b/pkg/fabric-client/mocks/mockpeer.go @@ -86,7 +86,7 @@ func (p *MockPeer) URL() string { } // ProcessTransactionProposal does not send anything anywhere but returns an empty mock ProposalResponse -func (p *MockPeer) ProcessTransactionProposal(tp apifabclient.TransactionProposal) (apifabclient.TransactionProposalResult, error) { +func (p *MockPeer) ProcessTransactionProposal(tp apifabclient.TransactionProposal) (apifabclient.TransactionProposalResponse, error) { if p.RWLock != nil { p.RWLock.Lock() defer p.RWLock.Unlock() @@ -98,12 +98,12 @@ func (p *MockPeer) ProcessTransactionProposal(tp apifabclient.TransactionProposa sID := &msp.SerializedIdentity{Mspid: "Org1MSP", IdBytes: []byte(certPem)} endorser, err := proto.Marshal(sID) if err != nil { - return apifabclient.TransactionProposalResult{}, err + return apifabclient.TransactionProposalResponse{}, err } p.Endorser = endorser } - return apifabclient.TransactionProposalResult{ + return apifabclient.TransactionProposalResponse{ Endorser: p.MockURL, Proposal: tp, Status: p.Status, diff --git a/pkg/fabric-client/mocks/mockresource.go b/pkg/fabric-client/mocks/mockresource.go index 07a078fd73..a4dc2dff5c 100644 --- a/pkg/fabric-client/mocks/mockresource.go +++ b/pkg/fabric-client/mocks/mockresource.go @@ -93,12 +93,9 @@ func (c *MockResource) InstallChaincode(req fab.InstallChaincodeRequest) ([]*fab } if req.Name == "errorInResponse" { - result := fab.TransactionProposalResult{Endorser: "http://peer1.com", Status: 10} - response := &fab.TransactionProposalResponse{TransactionProposalResult: result, Err: errors.New("Generate Response Error")} - return []*fab.TransactionProposalResponse{response}, "1234", nil + return []*fab.TransactionProposalResponse{}, "1234", errors.New("Generate Response Error") } - result := fab.TransactionProposalResult{Endorser: "http://peer1.com", Status: 0} - response := &fab.TransactionProposalResponse{TransactionProposalResult: result} + response := &fab.TransactionProposalResponse{Endorser: "http://peer1.com", Status: 0} return []*fab.TransactionProposalResponse{response}, "1234", nil } diff --git a/pkg/fabric-client/peer/deprecated_test.go b/pkg/fabric-client/peer/deprecated_test.go index 88a0615d4f..567caa2cb2 100644 --- a/pkg/fabric-client/peer/deprecated_test.go +++ b/pkg/fabric-client/peer/deprecated_test.go @@ -178,7 +178,7 @@ func TestDeprecatedProposalProcessorSendProposal(t *testing.T) { proc := mock_fab.NewMockProposalProcessor(mockCtrl) tp := mockTransactionProposal() - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(tp).Return(tpr, nil) diff --git a/pkg/fabric-client/peer/peer.go b/pkg/fabric-client/peer/peer.go index 56e3ad9507..86c4577599 100644 --- a/pkg/fabric-client/peer/peer.go +++ b/pkg/fabric-client/peer/peer.go @@ -187,7 +187,7 @@ func (p *Peer) URL() string { } // ProcessTransactionProposal sends the created proposal to peer for endorsement. -func (p *Peer) ProcessTransactionProposal(proposal fab.TransactionProposal) (fab.TransactionProposalResult, error) { +func (p *Peer) ProcessTransactionProposal(proposal fab.TransactionProposal) (fab.TransactionProposalResponse, error) { return p.processor.ProcessTransactionProposal(proposal) } diff --git a/pkg/fabric-client/peer/peer_test.go b/pkg/fabric-client/peer/peer_test.go index f63dd022e2..65f3a2ed90 100644 --- a/pkg/fabric-client/peer/peer_test.go +++ b/pkg/fabric-client/peer/peer_test.go @@ -190,7 +190,7 @@ func TestProposalProcessorSendProposal(t *testing.T) { proc := mock_fab.NewMockProposalProcessor(mockCtrl) tp := mockTransactionProposal() - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(tp).Return(tpr, nil) diff --git a/pkg/fabric-client/peer/peerendorser.go b/pkg/fabric-client/peer/peerendorser.go index 5a368d836d..02cd2df737 100644 --- a/pkg/fabric-client/peer/peerendorser.go +++ b/pkg/fabric-client/peer/peerendorser.go @@ -62,19 +62,19 @@ func newPeerEndorser(target string, certificate *x509.Certificate, serverHostOve } // ProcessTransactionProposal sends the transaction proposal to a peer and returns the response. -func (p *peerEndorser) ProcessTransactionProposal(proposal apifabclient.TransactionProposal) (apifabclient.TransactionProposalResult, error) { +func (p *peerEndorser) ProcessTransactionProposal(proposal apifabclient.TransactionProposal) (apifabclient.TransactionProposalResponse, error) { logger.Debugf("Processing proposal using endorser :%s", p.target) proposalResponse, err := p.sendProposal(proposal) if err != nil { - return apifabclient.TransactionProposalResult{ + return apifabclient.TransactionProposalResponse{ Proposal: proposal, Endorser: p.target, }, errors.Wrapf(err, "Transaction processor (%s) returned error for txID '%s'", p.target, proposal.TxnID.ID) } - return apifabclient.TransactionProposalResult{ + return apifabclient.TransactionProposalResponse{ Proposal: proposal, ProposalResponse: proposalResponse, Endorser: p.target, // TODO: what format is expected for Endorser? Just target? URL? diff --git a/pkg/fabric-client/peer/peerendorser_test.go b/pkg/fabric-client/peer/peerendorser_test.go index e313d30d0d..130a1a09b7 100644 --- a/pkg/fabric-client/peer/peerendorser_test.go +++ b/pkg/fabric-client/peer/peerendorser_test.go @@ -260,7 +260,7 @@ func TestProcessProposalGoodDial(t *testing.T) { } } -func testProcessProposal(t *testing.T, url string) (apifabclient.TransactionProposalResult, error) { +func testProcessProposal(t *testing.T, url string) (apifabclient.TransactionProposalResponse, error) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() config := mock_apiconfig.DefaultMockConfig(mockCtrl) diff --git a/pkg/fabric-client/resource/resource.go b/pkg/fabric-client/resource/resource.go index 5e33e1a837..11be9edb59 100644 --- a/pkg/fabric-client/resource/resource.go +++ b/pkg/fabric-client/resource/resource.go @@ -9,7 +9,6 @@ package resource import ( "net/http" - "strings" "time" "github.com/golang/protobuf/proto" @@ -20,6 +19,7 @@ import ( fcutils "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric/common/util" ab "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric/protos/orderer" ccomm "github.com/hyperledger/fabric-sdk-go/pkg/config/comm" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/txn" "github.com/hyperledger/fabric-sdk-go/pkg/logging" "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/common" @@ -433,36 +433,19 @@ func (c *Resource) InstallChaincode(req fab.InstallChaincodeRequest) ([]*fab.Tra return transactionProposalResponse, txID, err } -// MultiError represents a slice of errors originating from each target peer. -type MultiError []error - -func (me MultiError) Error() string { - msg := []string{} - for _, e := range me { - msg = append(msg, e.Error()) - } - return strings.Join(msg, ",") -} - func (c *Resource) queryChaincode(request fab.ChaincodeInvokeRequest, targets []fab.ProposalProcessor) ([][]byte, error) { - errors := MultiError{} + var errors multi.Errors responses := [][]byte{} - isErr := false for _, target := range targets { resp, err := c.queryChaincodeWithTarget(request, target) - responses = append(responses, resp) - errors = append(errors, err) - if err != nil { - isErr = true + errors = append(errors, err) } } - if isErr { - return responses, errors - } - return responses, nil + + return responses, errors.ToError() } func (c *Resource) queryChaincodeWithTarget(request fab.ChaincodeInvokeRequest, target fab.ProposalProcessor) ([]byte, error) { @@ -489,10 +472,6 @@ func (c *Resource) queryChaincodeWithTarget(request fab.ChaincodeInvokeRequest, } func validateResponse(response *fab.TransactionProposalResponse) error { - if response.Err != nil { - return errors.Errorf("error from %s (%s)", response.Endorser, response.Err.Error()) - } - if response.Status != http.StatusOK { return errors.Errorf("bad status from %s (%d)", response.Endorser, response.Status) } diff --git a/pkg/fabric-client/txn/proposal.go b/pkg/fabric-client/txn/proposal.go index 7700b87615..78fa164aca 100644 --- a/pkg/fabric-client/txn/proposal.go +++ b/pkg/fabric-client/txn/proposal.go @@ -13,6 +13,7 @@ import ( "github.com/pkg/errors" fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/common" pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" @@ -106,8 +107,6 @@ func SignProposal(ctx context, proposal *pb.Proposal) (*pb.SignedProposal, error } // SendProposal sends a TransactionProposal to ProposalProcessor. -// -// TODO: Refactor error out of struct and into multi error (eg type ResponseErrors []error) func SendProposal(proposal *fab.TransactionProposal, targets []fab.ProposalProcessor) ([]*fab.TransactionProposalResponse, error) { if proposal == nil || proposal.SignedProposal == nil { @@ -121,26 +120,28 @@ func SendProposal(proposal *fab.TransactionProposal, targets []fab.ProposalProce var responseMtx sync.Mutex var transactionProposalResponses []*fab.TransactionProposalResponse var wg sync.WaitGroup + errs := multi.Errors{} for _, p := range targets { wg.Add(1) go func(processor fab.ProposalProcessor) { defer wg.Done() - r, err := processor.ProcessTransactionProposal(*proposal) + resp, err := processor.ProcessTransactionProposal(*proposal) if err != nil { logger.Debugf("Received error response from txn proposal processing: %v", err) - // Error is handled downstream. + responseMtx.Lock() + errs = append(errs, err) + responseMtx.Unlock() + return } - tpr := fab.TransactionProposalResponse{ - TransactionProposalResult: r, Err: err} - responseMtx.Lock() - transactionProposalResponses = append(transactionProposalResponses, &tpr) + transactionProposalResponses = append(transactionProposalResponses, &resp) responseMtx.Unlock() }(p) } wg.Wait() - return transactionProposalResponses, nil + + return transactionProposalResponses, errs.ToError() } diff --git a/pkg/fabric-client/txn/proposal_test.go b/pkg/fabric-client/txn/proposal_test.go index 4cccf90299..81c731b0f7 100644 --- a/pkg/fabric-client/txn/proposal_test.go +++ b/pkg/fabric-client/txn/proposal_test.go @@ -17,6 +17,7 @@ import ( fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" "github.com/hyperledger/fabric-sdk-go/api/apifabclient/mocks" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/mocks" pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" ) @@ -159,7 +160,7 @@ func TestSendTransactionProposalToProcessors(t *testing.T) { tp := fab.TransactionProposal{ SignedProposal: &pb.SignedProposal{}, } - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 99, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(tp).Return(tpr, nil) targets := []fab.ProposalProcessor{proc} @@ -202,20 +203,25 @@ func TestProposalResponseError(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() proc := mock_apifabclient.NewMockProposalProcessor(mockCtrl) + proc2 := mock_apifabclient.NewMockProposalProcessor(mockCtrl) tp := fab.TransactionProposal{ SignedProposal: &pb.SignedProposal{}, } // Test with error from lower layer - tpr := fab.TransactionProposalResult{Endorser: "example.com", Status: 200, + tpr := fab.TransactionProposalResponse{Endorser: "example.com", Status: 200, Proposal: tp, ProposalResponse: nil} proc.EXPECT().ProcessTransactionProposal(tp).Return(tpr, testError) - targets := []fab.ProposalProcessor{proc} - resp, _ := SendProposal(&fab.TransactionProposal{ + proc2.EXPECT().ProcessTransactionProposal(tp).Return(tpr, testError) + + targets := []fab.ProposalProcessor{proc, proc2} + _, err := SendProposal(&fab.TransactionProposal{ SignedProposal: &pb.SignedProposal{}, }, targets) - assert.Equal(t, testError, resp[0].Err) + errs, ok := err.(multi.Errors) + assert.True(t, ok, "expected multi errors object") + assert.Equal(t, testError, errs[0]) } func setupMassiveTestPeers(numberOfPeers int) []fab.ProposalProcessor { diff --git a/pkg/fabric-client/txn/txn_test.go b/pkg/fabric-client/txn/txn_test.go index 99f1ac8a76..257f92d892 100644 --- a/pkg/fabric-client/txn/txn_test.go +++ b/pkg/fabric-client/txn/txn_test.go @@ -38,15 +38,13 @@ func TestNewTransaction(t *testing.T) { } test := &fab.TransactionProposalResponse{ - TransactionProposalResult: fab.TransactionProposalResult{ - Endorser: "http://peer1.com", - Proposal: fab.TransactionProposal{ - TxnID: txid, - Proposal: &pb.Proposal{Header: []byte("TEST"), Extension: []byte(""), Payload: []byte("")}, - SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, - }, - ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, + Endorser: "http://peer1.com", + Proposal: fab.TransactionProposal{ + TxnID: txid, + Proposal: &pb.Proposal{Header: []byte("TEST"), Extension: []byte(""), Payload: []byte("")}, + SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, }, + ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, } input := []*fab.TransactionProposalResponse{test} @@ -59,15 +57,13 @@ func TestNewTransaction(t *testing.T) { //Test invalid proposal payload scenario test = &fab.TransactionProposalResponse{ - TransactionProposalResult: fab.TransactionProposalResult{ - Endorser: "http://peer1.com", - Proposal: fab.TransactionProposal{ - TxnID: txid, - Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("TEST")}, - SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, - }, - ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, + Endorser: "http://peer1.com", + Proposal: fab.TransactionProposal{ + TxnID: txid, + Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("TEST")}, + SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, }, + ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, } input = []*fab.TransactionProposalResponse{test} @@ -79,14 +75,12 @@ func TestNewTransaction(t *testing.T) { //Test proposal response test = &fab.TransactionProposalResponse{ - TransactionProposalResult: fab.TransactionProposalResult{ - Endorser: "http://peer1.com", - Proposal: fab.TransactionProposal{ - Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("")}, - SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, TxnID: txid, - }, - ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, + Endorser: "http://peer1.com", + Proposal: fab.TransactionProposal{ + Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("")}, + SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, TxnID: txid, }, + ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 99, Payload: []byte("")}}, } input = []*fab.TransactionProposalResponse{test} @@ -99,14 +93,12 @@ func TestNewTransaction(t *testing.T) { //Test repeated field header nil scenario test = &fab.TransactionProposalResponse{ - TransactionProposalResult: fab.TransactionProposalResult{ - Endorser: "http://peer1.com", - Proposal: fab.TransactionProposal{ - Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("")}, - SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, TxnID: txid, - }, - ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 200, Payload: []byte("")}}, + Endorser: "http://peer1.com", + Proposal: fab.TransactionProposal{ + Proposal: &pb.Proposal{Header: []byte(""), Extension: []byte(""), Payload: []byte("")}, + SignedProposal: &pb.SignedProposal{Signature: []byte(""), ProposalBytes: []byte("")}, TxnID: txid, }, + ProposalResponse: &pb.ProposalResponse{Response: &pb.Response{Message: "success", Status: 200, Payload: []byte("")}}, } _, err = New([]*fab.TransactionProposalResponse{test}) diff --git a/pkg/fabric-txn/chclient/chclient.go b/pkg/fabric-txn/chclient/chclient.go index a5acb09c85..66cb94e9ee 100644 --- a/pkg/fabric-txn/chclient/chclient.go +++ b/pkg/fabric-txn/chclient/chclient.go @@ -15,6 +15,7 @@ import ( fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" "github.com/hyperledger/fabric-sdk-go/api/apitxn/chclient" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/errors/retry" "github.com/hyperledger/fabric-sdk-go/pkg/errors/status" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-txn/discovery" @@ -109,19 +110,25 @@ func (cc *ChannelClient) InvokeHandler(handler chclient.Handler, request chclien } } -func (cc *ChannelClient) resolveRetry(req *chclient.RequestContext, opts chclient.Opts) bool { - if !req.RetryHandler.Required(req.Error) { - return false +func (cc *ChannelClient) resolveRetry(ctx *chclient.RequestContext, opts chclient.Opts) bool { + errs, ok := ctx.Error.(multi.Errors) + if !ok { + errs = append(errs, ctx.Error) } - logger.Infof("Retrying on error %s", req.Error) + for _, e := range errs { + if ctx.RetryHandler.Required(e) { + logger.Infof("Retrying on error %s", e) + cc.greylist.Greylist(e) - cc.greylist.Greylist(req.Error) - // Reset context parameters - req.Opts.ProposalProcessors = opts.ProposalProcessors - req.Error = nil - req.Response = chclient.Response{} + // Reset context parameters + ctx.Opts.ProposalProcessors = opts.ProposalProcessors + ctx.Error = nil + ctx.Response = chclient.Response{} - return true + return true + } + } + return false } //prepareHandlerContexts prepares context objects for handlers @@ -150,7 +157,6 @@ func (cc *ChannelClient) prepareHandlerContexts(request chclient.Request, option } return requestContext, clientContext, nil - } //prepareOptsFromOptions Reads apitxn.Opts from chclient.Option array diff --git a/pkg/fabric-txn/chclient/chclient_test.go b/pkg/fabric-txn/chclient/chclient_test.go index 787a238f98..7cebac1609 100644 --- a/pkg/fabric-txn/chclient/chclient_test.go +++ b/pkg/fabric-txn/chclient/chclient_test.go @@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0 package chclient import ( + "fmt" "testing" "time" @@ -406,7 +407,7 @@ func TestExecuteTxWithRetries(t *testing.T) { testPeer1.Error = testStatus chClient := setupChannelClient([]apifabclient.Peer{testPeer1}, t) retryOpts := retry.DefaultOpts - retryOpts.Attempts = 1 + retryOpts.Attempts = 3 retryOpts.BackoffFactor = 1 retryOpts.InitialBackoff = retryInterval retryOpts.RetryableCodes = retry.ChannelClientRetryableCodes @@ -427,6 +428,26 @@ func TestExecuteTxWithRetries(t *testing.T) { assert.Equal(t, testResp, resp.Payload, "expected correct response") } +func TestMultiErrorPropogation(t *testing.T) { + testErr := fmt.Errorf("Test Error") + + testPeer1 := fcmocks.NewMockPeer("Peer1", "http://peer1.com") + testPeer1.Error = testErr + testPeer2 := fcmocks.NewMockPeer("Peer2", "http://peer2.com") + testPeer2.Error = testErr + chClient := setupChannelClient([]apifabclient.Peer{testPeer1, testPeer2}, t) + + _, err := chClient.Query(chclient.Request{ChaincodeID: "testCC", Fcn: "invoke", Args: [][]byte{[]byte("query"), []byte("b")}}) + if err == nil { + t.Fatalf("Should have failed for not success status") + } + statusError, ok := status.FromError(err) + assert.True(t, ok, "Expected status error") + assert.EqualValues(t, status.MultipleErrors, status.ToSDKStatusCode(statusError.Code)) + assert.Equal(t, status.ClientStatus, statusError.Group) + assert.Equal(t, "Multiple errors occurred: \nTest Error\nTest Error", statusError.Message, "Expected multi error message") +} + func TestDiscoveryGreylist(t *testing.T) { testPeer1 := fcmocks.NewMockPeer("Peer1", "http://peer1.com") testPeer1.Error = status.New(status.EndorserClientStatus, @@ -608,15 +629,5 @@ func createAndSendTransactionProposal(sender apifabclient.ProposalSender, chrequ TransientMap: chrequest.TransientMap, } - transactionProposalResponses, txnID, err := sender.SendTransactionProposal(request, targets) - if err != nil { - return nil, txnID, err - } - - for _, v := range transactionProposalResponses { - if v.Err != nil { - return nil, txnID, errors.WithMessage(v.Err, "SendTransactionProposal failed") - } - } - return transactionProposalResponses, txnID, nil + return sender.SendTransactionProposal(request, targets) } diff --git a/pkg/fabric-txn/resmgmtclient/resmgmt.go b/pkg/fabric-txn/resmgmtclient/resmgmt.go index a1d60802bc..b42c9f7df7 100644 --- a/pkg/fabric-txn/resmgmtclient/resmgmt.go +++ b/pkg/fabric-txn/resmgmtclient/resmgmt.go @@ -14,6 +14,7 @@ import ( fab "github.com/hyperledger/fabric-sdk-go/api/apifabclient" resmgmt "github.com/hyperledger/fabric-sdk-go/api/apitxn/resmgmtclient" + "github.com/hyperledger/fabric-sdk-go/pkg/errors/multi" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/peer" "github.com/hyperledger/fabric-sdk-go/pkg/fabric-client/txn" "github.com/hyperledger/fabric-sdk-go/pkg/fabsdk/api" @@ -270,16 +271,15 @@ func (rc *ResourceMgmtClient) InstallCC(req resmgmt.InstallCCRequest, options .. } responses := make([]resmgmt.InstallCCResponse, 0) + var errs multi.Errors // Targets will be adjusted if cc has already been installed newTargets := make([]fab.Peer, 0) - for _, target := range targets { installed, err := rc.isChaincodeInstalled(req, target) if err != nil { - // Add to responses with unable to verify error message - response := resmgmt.InstallCCResponse{Target: target.URL(), Err: errors.Errorf("unable to verify if cc is installed on %s", target.URL())} - responses = append(responses, response) + // Add to errors with unable to verify error message + errs = append(errs, errors.Errorf("unable to verify if cc is installed on %s", target.URL())) continue } if installed { @@ -300,20 +300,18 @@ func (rc *ResourceMgmtClient) InstallCC(req resmgmt.InstallCCRequest, options .. icr := fab.InstallChaincodeRequest{Name: req.Name, Path: req.Path, Version: req.Version, Package: req.Package, Targets: peer.PeersToTxnProcessors(newTargets)} transactionProposalResponse, _, err := rc.resource.InstallChaincode(icr) - if err != nil { - return nil, errors.WithMessage(err, "InstallChaincode failed") - } - for _, v := range transactionProposalResponse { + logger.Debugf("Install chaincode '%s' endorser '%s' returned ProposalResponse status:%v", req.Name, v.Endorser, v.Status) - logger.Debugf("Install chaincode '%s' endorser '%s' returned ProposalResponse status:%v, error:'%s'", req.Name, v.Endorser, v.Status, v.Err) - - response := resmgmt.InstallCCResponse{Target: v.Endorser, Status: v.Status, Err: v.Err} + response := resmgmt.InstallCCResponse{Target: v.Endorser, Status: v.Status} responses = append(responses, response) } - return responses, nil + if err != nil { + return responses, errors.WithMessage(err, "InstallChaincode failed") + } + return responses, nil } func checkRequiredInstallCCParams(req resmgmt.InstallCCRequest) error { @@ -398,12 +396,6 @@ func (rc *ResourceMgmtClient) sendCCProposal(ccProposalType CCProposalType, chan return errors.Errorf("chaincode proposal type %d not supported", ccProposalType) } - for _, v := range txProposalResponse { - if v.Err != nil { - return errors.WithMessage(v.Err, "cc proposal failed") - } - } - channelService, err := rc.channelProvider.NewChannelService(rc.identity, channelID) if err != nil { return errors.WithMessage(err, "Unable to get channel service") diff --git a/pkg/fabric-txn/resmgmtclient/resmgmt_test.go b/pkg/fabric-txn/resmgmtclient/resmgmt_test.go index 40992d2bbb..bbb87a0410 100644 --- a/pkg/fabric-txn/resmgmtclient/resmgmt_test.go +++ b/pkg/fabric-txn/resmgmtclient/resmgmt_test.go @@ -302,7 +302,8 @@ func TestInstallCCWithOpts(t *testing.T) { // Setup targets var peers []fab.Peer - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} + peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", + Status: 200, MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} peers = append(peers, &peer) // Already installed chaincode request @@ -345,13 +346,6 @@ func TestInstallCCWithOpts(t *testing.T) { if err == nil { t.Fatalf("Should have failed since install cc returns an error in the client") } - - // Chaincode that causes response error in installed chaincodes - req = resmgmt.InstallCCRequest{Name: "errorInResponse", Version: "v0", Path: "path", Package: &fab.CCPackage{Type: 1, Code: []byte("code")}} - responses, err = rc.InstallCC(req, resmgmt.WithTargets(peers...)) - if err != nil { - t.Fatal(err) - } } func TestInstallCC(t *testing.T) { diff --git a/pkg/fabric-txn/txnhandler/txnhandler.go b/pkg/fabric-txn/txnhandler/txnhandler.go index 6303dc7473..c455e1718c 100644 --- a/pkg/fabric-txn/txnhandler/txnhandler.go +++ b/pkg/fabric-txn/txnhandler/txnhandler.go @@ -249,18 +249,5 @@ func createAndSendTransactionProposal(sender apifabclient.ProposalSender, chrequ TransientMap: chrequest.TransientMap, } - transactionProposalResponses, txnID, err := sender.SendTransactionProposal(request, targets) - if err != nil { - return nil, txnID, err - } - - for _, v := range transactionProposalResponses { - if v.Err != nil { - logger.Debugf("SendTransactionProposal failed (%v, %s)", v.Endorser, v.Err.Error()) - return nil, txnID, errors.WithMessage(v.Err, "SendTransactionProposal failed") - } - logger.Debugf("invoke Endorser '%s' returned ProposalResponse status:%v", v.Endorser, v.Status) - } - - return transactionProposalResponses, txnID, nil + return sender.SendTransactionProposal(request, targets) } diff --git a/test/integration/base_test_setup.go b/test/integration/base_test_setup.go index b2719d0961..3c3c62071b 100644 --- a/test/integration/base_test_setup.go +++ b/test/integration/base_test_setup.go @@ -187,16 +187,10 @@ func (setup *BaseSetupImpl) InstallCC(name string, path string, version string, icr := fab.InstallChaincodeRequest{Name: name, Path: path, Version: version, Package: ccPackage, Targets: peer.PeersToTxnProcessors(setup.Channel.Peers())} - transactionProposalResponse, _, err := setup.Client.InstallChaincode(icr) - + _, _, err := setup.Client.InstallChaincode(icr) if err != nil { return errors.WithMessage(err, "InstallChaincode failed") } - for _, v := range transactionProposalResponse { - if v.Err != nil { - return errors.WithMessage(v.Err, "InstallChaincode endorser failed") - } - } return nil } @@ -255,12 +249,6 @@ func CreateAndSendTransactionProposal(channel fab.Channel, chainCodeID string, return nil, txnID, err } - for _, v := range transactionProposalResponses { - if v.Err != nil { - return nil, txnID, errors.Wrapf(v.Err, "endorser %s failed", v.Endorser) - } - } - return transactionProposalResponses, txnID, nil }