Skip to content

Commit 916b42a

Browse files
author
Nao Nishijima
committed
[FAB-6387] Fix error code missing in endorser
Error code is not returned because response value is ignored. - ProcessProposal function in core/endorser/endorser.go returns a response and an error value if an error happens. - After that, a response and an error value are passed to ProcessProposal function in protos/peer/peer.bp.go. - This function only returns the error value. Even though response value is set, the response value is ignored. This CRs fixes this problem by changing returned value. - ProposalResponse function returns only response value even if an error happens. - We can get error code because an error value is included in response value. Change-Id: If09da624478729969eea3041cc27af23a902ba39 Signed-off-by: Nao Nishijima <nao.nishijima@hal.hitachi.com>
1 parent 72ef358 commit 916b42a

File tree

8 files changed

+82
-70
lines changed

8 files changed

+82
-70
lines changed

‎core/endorser/endorser.go

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,6 @@ import (
2525
"golang.org/x/net/context"
2626
)
2727

28-
// >>>>> begin errors section >>>>>
29-
//chaincodeError is a fabric error signifying error from chaincode
30-
type chaincodeError struct {
31-
status int32
32-
msg string
33-
}
34-
35-
func (ce chaincodeError) Error() string {
36-
return fmt.Sprintf("chaincode error (status: %d, message: %s)", ce.status, ce.msg)
37-
}
38-
39-
// <<<<< end errors section <<<<<<
40-
4128
var endorserLogger = flogging.MustGetLogger("endorser")
4229

4330
// The Jira issue that documents Endorser flow along with its relationship to
@@ -423,7 +410,9 @@ func (e *Endorser) preProcess(signedProp *pb.SignedProposal) (*validateResult, e
423410
if chainID != "" {
424411
// here we handle uniqueness check and ACLs for proposals targeting a chain
425412
if _, err = e.s.GetTransactionByID(chainID, txid); err == nil {
426-
return vr, errors.Errorf("duplicate transaction found [%s]. Creator [%x]", txid, shdr.Creator)
413+
err = errors.Errorf("duplicate transaction found [%s]. Creator [%x]", txid, shdr.Creator)
414+
vr.resp = &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}
415+
return vr, err
427416
}
428417

429418
// check ACL only for application chaincodes; ACLs
@@ -468,10 +457,10 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
468457
var historyQueryExecutor ledger.HistoryQueryExecutor
469458
if chainID != "" {
470459
if txsim, err = e.s.GetTxSimulator(chainID, txid); err != nil {
471-
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
460+
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
472461
}
473462
if historyQueryExecutor, err = e.s.GetHistoryQueryExecutor(chainID); err != nil {
474-
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
463+
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
475464
}
476465
// Add the historyQueryExecutor to context
477466
// TODO shouldn't we also add txsim to context here as well? Rather than passing txsim parameter
@@ -490,7 +479,7 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
490479
//1 -- simulate
491480
cd, res, simulationResult, ccevent, err := e.simulateProposal(ctx, chainID, txid, signedProp, prop, hdrExt.ChaincodeId, txsim)
492481
if err != nil {
493-
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
482+
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
494483
}
495484
if res != nil {
496485
if res.Status >= shim.ERROR {
@@ -504,10 +493,10 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
504493
}
505494
pResp, err := putils.CreateProposalResponseFailure(prop.Header, prop.Payload, res, simulationResult, cceventBytes, hdrExt.ChaincodeId, hdrExt.PayloadVisibility)
506495
if err != nil {
507-
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
496+
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
508497
}
509498

510-
return pResp, &chaincodeError{res.Status, res.Message}
499+
return pResp, nil
511500
}
512501
}
513502

@@ -521,20 +510,18 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
521510
} else {
522511
pResp, err = e.endorseProposal(ctx, chainID, txid, signedProp, prop, res, simulationResult, ccevent, hdrExt.PayloadVisibility, hdrExt.ChaincodeId, txsim, cd)
523512
if err != nil {
524-
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
513+
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, nil
525514
}
526-
if pResp != nil {
527-
if res.Status >= shim.ERRORTHRESHOLD {
528-
endorserLogger.Debugf("[%s][%s] endorseProposal() resulted in chaincode %s error for txid: %s", chainID, shorttxid(txid), hdrExt.ChaincodeId, txid)
529-
return pResp, &chaincodeError{res.Status, res.Message}
530-
}
515+
if pResp.Response.Status >= shim.ERRORTHRESHOLD {
516+
endorserLogger.Debugf("[%s][%s] endorseProposal() resulted in chaincode %s error for txid: %s", chainID, shorttxid(txid), hdrExt.ChaincodeId, txid)
517+
return pResp, nil
531518
}
532519
}
533520

534521
// Set the proposal response payload - it
535522
// contains the "return value" from the
536523
// chaincode invocation
537-
pResp.Response.Payload = res.Payload
524+
pResp.Response = res
538525

539526
return pResp, nil
540527
}

‎core/endorser/endorser_test.go

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ func TestEndorserNilProp(t *testing.T) {
6767
GetTxSimulatorRv: &ccprovider.MockTxSim{&ledger.TxSimulationResults{PubSimulationResults: &rwset.TxReadWriteSet{}}},
6868
})
6969

70-
_, err := es.ProcessProposal(context.Background(), nil)
70+
pResp, err := es.ProcessProposal(context.Background(), nil)
7171
assert.Error(t, err)
72+
assert.EqualValues(t, 500, pResp.Response.Status)
73+
assert.Equal(t, "nil arguments", pResp.Response.Message)
7274
}
7375

7476
func TestEndorserUninvokableSysCC(t *testing.T) {
@@ -83,8 +85,10 @@ func TestEndorserUninvokableSysCC(t *testing.T) {
8385

8486
signedProp := getSignedProp("ccid", "0", t)
8587

86-
_, err := es.ProcessProposal(context.Background(), signedProp)
88+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
8789
assert.Error(t, err)
90+
assert.EqualValues(t, 500, pResp.Response.Status)
91+
assert.Equal(t, "chaincode ccid cannot be invoked through a proposal", pResp.Response.Message)
8892
}
8993

9094
func TestEndorserCCInvocationFailed(t *testing.T) {
@@ -95,14 +99,16 @@ func TestEndorserCCInvocationFailed(t *testing.T) {
9599
GetApplicationConfigRv: &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
96100
GetTransactionByIDErr: errors.New(""),
97101
ChaincodeDefinitionRv: &resourceconfig.MockChaincodeDefinition{EndorsementStr: "ESCC"},
98-
ExecuteResp: &pb.Response{Status: 1000, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}})},
102+
ExecuteResp: &pb.Response{Status: 1000, Payload: utils.MarshalOrPanic(&pb.ProposalResponse{Response: &pb.Response{}}), Message: "Chaincode Error"},
99103
GetTxSimulatorRv: &ccprovider.MockTxSim{&ledger.TxSimulationResults{PubSimulationResults: &rwset.TxReadWriteSet{}}},
100104
})
101105

102106
signedProp := getSignedProp("ccid", "0", t)
103107

104-
_, err := es.ProcessProposal(context.Background(), signedProp)
105-
assert.Error(t, err)
108+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
109+
assert.NoError(t, err)
110+
assert.EqualValues(t, 1000, pResp.Response.Status)
111+
assert.Regexp(t, "Chaincode Error", pResp.Response.Message)
106112
}
107113

108114
func TestEndorserNoCCDef(t *testing.T) {
@@ -119,8 +125,10 @@ func TestEndorserNoCCDef(t *testing.T) {
119125

120126
signedProp := getSignedProp("ccid", "0", t)
121127

122-
_, err := es.ProcessProposal(context.Background(), signedProp)
123-
assert.Error(t, err)
128+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
129+
assert.NoError(t, err)
130+
assert.EqualValues(t, 500, pResp.Response.Status)
131+
assert.Regexp(t, "make sure the chaincode", pResp.Response.Message)
124132
}
125133

126134
func TestEndorserBadInstPolicy(t *testing.T) {
@@ -138,8 +146,9 @@ func TestEndorserBadInstPolicy(t *testing.T) {
138146

139147
signedProp := getSignedProp("ccid", "0", t)
140148

141-
_, err := es.ProcessProposal(context.Background(), signedProp)
142-
assert.Error(t, err)
149+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
150+
assert.NoError(t, err)
151+
assert.EqualValues(t, 500, pResp.Response.Status)
143152
}
144153

145154
func TestEndorserSysCC(t *testing.T) {
@@ -157,8 +166,9 @@ func TestEndorserSysCC(t *testing.T) {
157166

158167
signedProp := getSignedProp("ccid", "0", t)
159168

160-
_, err := es.ProcessProposal(context.Background(), signedProp)
169+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
161170
assert.NoError(t, err)
171+
assert.EqualValues(t, 200, pResp.Response.Status)
162172
}
163173

164174
func TestEndorserCCInvocationError(t *testing.T) {
@@ -175,8 +185,9 @@ func TestEndorserCCInvocationError(t *testing.T) {
175185

176186
signedProp := getSignedProp("ccid", "0", t)
177187

178-
_, err := es.ProcessProposal(context.Background(), signedProp)
179-
assert.Error(t, err)
188+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
189+
assert.NoError(t, err)
190+
assert.EqualValues(t, 500, pResp.Response.Status)
180191
}
181192

182193
func TestEndorserLSCCBadType(t *testing.T) {
@@ -201,8 +212,10 @@ func TestEndorserLSCCBadType(t *testing.T) {
201212
)
202213
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
203214

204-
_, err := es.ProcessProposal(context.Background(), signedProp)
205-
assert.Error(t, err)
215+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
216+
assert.NoError(t, err)
217+
assert.EqualValues(t, 500, pResp.Response.Status)
218+
assert.Equal(t, "Unknown chaincodeType: UNDEFINED", pResp.Response.Message)
206219
}
207220

208221
func TestEndorserDupTXId(t *testing.T) {
@@ -218,8 +231,10 @@ func TestEndorserDupTXId(t *testing.T) {
218231

219232
signedProp := getSignedProp("ccid", "0", t)
220233

221-
_, err := es.ProcessProposal(context.Background(), signedProp)
234+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
222235
assert.Error(t, err)
236+
assert.EqualValues(t, 500, pResp.Response.Status)
237+
assert.Regexp(t, "duplicate transaction found", pResp.Response.Message)
223238
}
224239

225240
func TestEndorserBadACL(t *testing.T) {
@@ -237,8 +252,9 @@ func TestEndorserBadACL(t *testing.T) {
237252

238253
signedProp := getSignedProp("ccid", "0", t)
239254

240-
_, err := es.ProcessProposal(context.Background(), signedProp)
255+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
241256
assert.Error(t, err)
257+
assert.EqualValues(t, 500, pResp.Response.Status)
242258
}
243259

244260
func TestEndorserGoodPathEmptyChannel(t *testing.T) {
@@ -255,8 +271,9 @@ func TestEndorserGoodPathEmptyChannel(t *testing.T) {
255271

256272
signedProp := getSignedPropWithCHIdAndArgs("", "ccid", "0", [][]byte{[]byte("args")}, t)
257273

258-
_, err := es.ProcessProposal(context.Background(), signedProp)
274+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
259275
assert.NoError(t, err)
276+
assert.EqualValues(t, 200, pResp.Response.Status)
260277
}
261278

262279
func TestEndorserLSCCInitFails(t *testing.T) {
@@ -282,8 +299,9 @@ func TestEndorserLSCCInitFails(t *testing.T) {
282299
)
283300
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
284301

285-
_, err := es.ProcessProposal(context.Background(), signedProp)
286-
assert.Error(t, err)
302+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
303+
assert.NoError(t, err)
304+
assert.EqualValues(t, 500, pResp.Response.Status)
287305
}
288306

289307
func TestEndorserLSCCDeploySysCC(t *testing.T) {
@@ -312,8 +330,10 @@ func TestEndorserLSCCDeploySysCC(t *testing.T) {
312330
)
313331
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
314332

315-
_, err := es.ProcessProposal(context.Background(), signedProp)
316-
assert.Error(t, err)
333+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
334+
assert.NoError(t, err)
335+
assert.EqualValues(t, 500, pResp.Response.Status)
336+
assert.Equal(t, "attempting to deploy a system chaincode barf/testchainid", pResp.Response.Message)
317337
}
318338

319339
func TestEndorserLSCCJava1(t *testing.T) {
@@ -343,8 +363,10 @@ func TestEndorserLSCCJava1(t *testing.T) {
343363
)
344364
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
345365

346-
_, err := es.ProcessProposal(context.Background(), signedProp)
347-
assert.Error(t, err)
366+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
367+
assert.NoError(t, err)
368+
assert.EqualValues(t, 500, pResp.Response.Status)
369+
assert.Equal(t, "Java chaincode is work-in-progress and disabled", pResp.Response.Message)
348370
}
349371

350372
func TestEndorserLSCCJava2(t *testing.T) {
@@ -374,8 +396,9 @@ func TestEndorserLSCCJava2(t *testing.T) {
374396
)
375397
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
376398

377-
_, err := es.ProcessProposal(context.Background(), signedProp)
378-
assert.Error(t, err)
399+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
400+
assert.NoError(t, err)
401+
assert.EqualValues(t, 500, pResp.Response.Status)
379402
}
380403

381404
func TestEndorserGoodPathWEvents(t *testing.T) {
@@ -393,8 +416,9 @@ func TestEndorserGoodPathWEvents(t *testing.T) {
393416

394417
signedProp := getSignedProp("ccid", "0", t)
395418

396-
_, err := es.ProcessProposal(context.Background(), signedProp)
419+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
397420
assert.NoError(t, err)
421+
assert.EqualValues(t, 200, pResp.Response.Status)
398422
}
399423

400424
func TestEndorserBadChannel(t *testing.T) {
@@ -411,8 +435,10 @@ func TestEndorserBadChannel(t *testing.T) {
411435

412436
signedProp := getSignedPropWithCHID("ccid", "0", "barfchain", t)
413437

414-
_, err := es.ProcessProposal(context.Background(), signedProp)
438+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
415439
assert.Error(t, err)
440+
assert.EqualValues(t, 500, pResp.Response.Status)
441+
assert.Equal(t, "access denied: channel [barfchain] creator org [SampleOrg]", pResp.Response.Message)
416442
}
417443

418444
func TestEndorserGoodPath(t *testing.T) {
@@ -429,8 +455,9 @@ func TestEndorserGoodPath(t *testing.T) {
429455

430456
signedProp := getSignedProp("ccid", "0", t)
431457

432-
_, err := es.ProcessProposal(context.Background(), signedProp)
458+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
433459
assert.NoError(t, err)
460+
assert.EqualValues(t, 200, pResp.Response.Status)
434461
}
435462

436463
func TestEndorserLSCC(t *testing.T) {
@@ -455,8 +482,9 @@ func TestEndorserLSCC(t *testing.T) {
455482
)
456483
signedProp := getSignedPropWithCHIdAndArgs(util.GetTestChainID(), "lscc", "0", [][]byte{[]byte("deploy"), []byte("a"), cds}, t)
457484

458-
_, err := es.ProcessProposal(context.Background(), signedProp)
485+
pResp, err := es.ProcessProposal(context.Background(), signedProp)
459486
assert.NoError(t, err)
487+
assert.EqualValues(t, 200, pResp.Response.Status)
460488
}
461489

462490
func TestSimulateProposal(t *testing.T) {
@@ -501,11 +529,6 @@ func TestEndorserJavaChecks(t *testing.T) {
501529
assert.Error(t, err)
502530
}
503531

504-
func TestChaincodeError_Error(t *testing.T) {
505-
ce := &chaincodeError{status: 1, msg: "foo"}
506-
assert.Equal(t, ce.Error(), "chaincode error (status: 1, message: foo)")
507-
}
508-
509532
var signer msp.SigningIdentity
510533

511534
func TestMain(m *testing.M) {

‎core/scc/escc/endorser_onevalidsignature.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (e *EndorserOneValidSignature) Invoke(stub shim.ChaincodeStubInterface) pb.
112112
}
113113

114114
if response.Status >= shim.ERRORTHRESHOLD {
115-
return shim.Error(fmt.Sprintf("Status code less than %d will be endorsed, received status code: %d", shim.ERRORTHRESHOLD, response.Status))
115+
return *response
116116
}
117117

118118
// handle simulation results

‎core/scc/escc/endorser_onevalidsignature_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,14 @@ func TestInvoke(t *testing.T) {
159159
// Failed path: status code = 500
160160
args = [][]byte{[]byte("test"), []byte("test"), []byte("test"), ccidBytes, failRes, []byte("test")}
161161
res := stub.MockInvoke("1", args)
162-
assert.NotEqual(t, res.Status, shim.OK, "Invoke should have failed with status code: %d", ccFailResponse.Status)
163-
assert.Contains(t, res.Message, fmt.Sprintf("Status code less than %d will be endorsed", shim.ERRORTHRESHOLD))
162+
assert.NotEqual(t, res.Status, shim.OK, "Invoke should have failed with status code: %d", failResponse.Status)
163+
assert.Equal(t, res, *failResponse)
164164

165165
// Failed path: status code = 400
166166
args = [][]byte{[]byte("test"), []byte("test"), []byte("test"), ccidBytes, ccFailRes, []byte("test")}
167167
res = stub.MockInvoke("1", args)
168168
assert.NotEqual(t, res.Status, shim.OK, "Invoke should have failed with status code: %d", ccFailResponse.Status)
169-
assert.Contains(t, res.Message, fmt.Sprintf("Status code less than %d will be endorsed", shim.ERRORTHRESHOLD))
169+
assert.Equal(t, res, *ccFailResponse)
170170

171171
// Successful path - create a proposal
172172
cs := &pb.ChaincodeSpec{

‎peer/chaincode/common.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func chaincodeInvokeOrQuery(cmd *cobra.Command, invoke bool, cf *ChaincodeCmdFac
114114
}
115115

116116
if invoke {
117-
if proposalResp.Response.Status >= shim.ERROR {
117+
if proposalResp.Response.Status >= shim.ERRORTHRESHOLD {
118118
logger.Debugf("ESCC invoke result: %v", proposalResp)
119119
pRespPayload, err := putils.GetProposalResponsePayload(proposalResp.Payload)
120120
if err != nil {
@@ -140,6 +140,8 @@ func chaincodeInvokeOrQuery(cmd *cobra.Command, invoke bool, cf *ChaincodeCmdFac
140140
} else {
141141
if proposalResp == nil {
142142
return fmt.Errorf("error query %s by endorsing: %s", chainFuncName, err)
143+
} else if proposalResp.Response.Status >= shim.ERRORTHRESHOLD {
144+
return fmt.Errorf("error query %s by endorsing: %s", chainFuncName, proposalResp.Response)
143145
}
144146

145147
if chaincodeQueryRaw && chaincodeQueryHex {
@@ -452,7 +454,7 @@ func ChaincodeInvokeOrQuery(
452454

453455
if invoke {
454456
if proposalResp != nil {
455-
if proposalResp.Response.Status >= shim.ERROR {
457+
if proposalResp.Response.Status >= shim.ERRORTHRESHOLD {
456458
return proposalResp, nil
457459
}
458460
// assemble a signed transaction (it's an Envelope message)

0 commit comments

Comments
 (0)