FABN-1239: Better error on submitTransaction failure

Improve the error message produced when
Transaction.submitTransaction fails because of bad proposal
responses. Include the peer name and response status in the
error message.

Change-Id: I683a7686f39668ddfdbe55955431c1a5203f4977
Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>

fabric-network/lib/transaction.js

@@ -191,19 +191,15 @@ class Transaction {
 		}
 
 		const validResponses = [];
-		const invalidResponses = [];
-		const invalidResponseMsgs = [];
+		const errorResponses = [];
 
 		responses.forEach((responseContent) => {
 			if (responseContent instanceof Error) {
 				// this is either an error from the sdk, peer response or chaincode response.
 				// we can distinguish between sdk vs peer/chaincode by the isProposalResponse flag in the future.
 				// TODO: would be handy to know which peer the response is from and include it here.
-				const message = responseContent.message;
 				logger.warn('_validatePeerResponses: Received error response from peer:', responseContent);
-				logger.debug('_validatePeerResponses: invalid response from peer %j', responseContent.peer);
-				invalidResponseMsgs.push(message);
-				invalidResponses.push(responseContent);
+				errorResponses.push(responseContent);
 			} else {
 				// anything else is a successful response ie status will be less then 400.
 				// in the future we can do things like verifyProposalResponse and compareProposalResponseResults
@@ -215,14 +211,16 @@ class Transaction {
 		});
 
 		if (validResponses.length === 0) {
-			const messages = Array.of(`No valid responses from any peers. ${invalidResponseMsgs.length} peer error responses:`,
-				...invalidResponseMsgs);
+			const errorMessages = errorResponses.map((response) => util.format('peer=%s, status=%s, message=%s',
+				response.peer.name, response.status, response.message));
+			const messages = Array.of(`No valid responses from any peers. ${errorResponses.length} peer error responses:`,
+				...errorMessages);
 			const msg = messages.join('\n    ');
 			logger.error('_validatePeerResponses: ' + msg);
 			throw new Error(msg);
 		}
 
-		return {validResponses, invalidResponses};
+		return {validResponses, invalidResponses: errorResponses};
 	}
 
 	/**

fabric-network/test/transaction.js

@@ -24,26 +24,35 @@ describe('Transaction', () => {
 	const chaincodeId = 'chaincode-id';
 	const expectedResult = Buffer.from('42');
 
+	const peerInfo = {name: 'PEER_NAME', url: 'grpc://fakehost:9999'};
 	const fakeProposal = {proposal: 'I do'};
 	const fakeHeader = {header: 'gooooal'};
 	const validProposalResponse = {
 		response: {
 			status: 200,
 			payload: expectedResult,
-			peer: {url: 'grpc://fakehost:9999'}
+			peer: peerInfo
 		}
 	};
 	const noPayloadProposalResponse = {
 		response: {
-			status: 200
+			status: 200,
+			peer: peerInfo
 		}
 	};
 	const errorResponseMessage = 'I_AM_AN_ERROR_RESPONSE';
-	const errorProposalResponse = Object.assign(new Error(errorResponseMessage), {response: {status: 500, payload: 'error', peer: {url: 'grpc://fakehost:9999'}}});
+	// Note that error responses have the 'response' property of the actual proposal response promoted up to the
+	// top-level error object in Peer.sendProposal()
+	const errorProposalResponse = Object.assign(new Error(errorResponseMessage), {
+		status: 500,
+		payload: 'error',
+		peer: peerInfo
+	});
 	const emptyStringProposalResponse = {
 		response: {
 			status: 200,
-			payload: Buffer.from('')
+			payload: Buffer.from(''),
+			peer: peerInfo
 		}
 	};
 
@@ -180,6 +189,18 @@ describe('Transaction', () => {
 			return expect(promise).to.be.rejectedWith(errorResponseMessage);
 		});
 
+		it('throws with message including underlying error status', () => {
+			channel.sendTransactionProposal.resolves(errorProposalResponses);
+			const promise = transaction.submit();
+			return expect(promise).to.be.rejectedWith(`${errorProposalResponse.status}`);
+		});
+
+		it('throws with message including underlying error peer name', () => {
+			channel.sendTransactionProposal.resolves(errorProposalResponses);
+			const promise = transaction.submit();
+			return expect(promise).to.be.rejectedWith(`${errorProposalResponse.peer.name}`);
+		});
+
 		it('succeeds if some proposal responses are valid', () => {
 			channel.sendTransactionProposal.resolves(mixedProposalResponses);
 			const promise = transaction.submit();

package.json

@@ -23,7 +23,7 @@
     "test:protos": "npm run coverage -- fabric-protos/test",
     "test:cucumber": "cucumber-js ./test/scenario/features/*.feature",
     "test:all": "nyc npm run unit-test:all",
-    "unit-test:all": "npm run unit-test -- fabric-ca-client/test && npm run unit-test -- fabric-client/test && npm run unit-test -- fabric-network/test",
+    "unit-test:all": "npm run unit-test -- fabric-common/test && npm run unit-test -- fabric-ca-client/test && npm run unit-test -- fabric-client/test && npm run unit-test -- fabric-network/test",
     "coverage": "nyc npm run unit-test",
     "unit-test": "mocha --exclude 'fabric-client/test/data/**' --recursive",
     "compile": "tsc --project test/typescript",