From 60aedf1b82e31772a43a9f93f2e7f04fd11a18ff Mon Sep 17 00:00:00 2001 From: James Taylor Date: Thu, 15 Jul 2021 18:30:38 +0100 Subject: [PATCH] Refactor transaction logic Remove duplication and handle errors from the asset transfer smart contract Signed-off-by: James Taylor --- .gitignore | 1 + .../rest-api-typescript/src/assets.router.ts | 225 +++++++----------- .../rest-api-typescript/src/errors.ts | 33 +++ .../rest-api-typescript/src/fabric.ts | 141 ++++++----- .../rest-api-typescript/src/redis.ts | 58 ++++- 5 files changed, 266 insertions(+), 192 deletions(-) create mode 100644 .gitignore create mode 100644 asset-transfer-basic/rest-api-typescript/src/errors.ts diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..b0b21603 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +local.http diff --git a/asset-transfer-basic/rest-api-typescript/src/assets.router.ts b/asset-transfer-basic/rest-api-typescript/src/assets.router.ts index 4a950636..f70db1e3 100644 --- a/asset-transfer-basic/rest-api-typescript/src/assets.router.ts +++ b/asset-transfer-basic/rest-api-typescript/src/assets.router.ts @@ -15,15 +15,18 @@ import { body, validationResult } from 'express-validator'; import { Contract } from 'fabric-network'; import { getReasonPhrase, StatusCodes } from 'http-status-codes'; import { Redis } from 'ioredis'; -import { - clearTransactionDetails, - createDeferredEventHandler, - storeTransactionDetails, -} from './fabric'; +import { AssetExistsError, AssetNotFoundError } from './errors'; +import { evatuateTransaction, submitTransaction } from './fabric'; import { logger } from './logger'; -const { ACCEPTED, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_FOUND, OK } = - StatusCodes; +const { + ACCEPTED, + BAD_REQUEST, + CONFLICT, + INTERNAL_SERVER_ERROR, + NOT_FOUND, + OK, +} = StatusCodes; export const assetsRouter = express.Router(); @@ -33,7 +36,7 @@ assetsRouter.get('/', async (req: Request, res: Response) => { try { const contract: Contract = req.app.get('contract'); - const data = await contract.evaluateTransaction('GetAllAssets'); + const data = await evatuateTransaction(contract, 'GetAllAssets'); const assets = JSON.parse(data.toString()); return res.status(OK).json(assets); @@ -61,6 +64,7 @@ assetsRouter.post( if (!errors.isEmpty()) { return res.status(BAD_REQUEST).json({ status: getReasonPhrase(BAD_REQUEST), + reason: 'VALIDATION_ERROR', message: 'Invalid request body', timestamp: new Date().toISOString(), errors: errors.array(), @@ -69,27 +73,14 @@ assetsRouter.post( const contract: Contract = req.app.get('contract'); const redis: Redis = req.app.get('redis'); - const txn = contract.createTransaction('CreateAsset'); - const txnId = txn.getTransactionId(); - const txnState = txn.serialize(); - const txnArgs = JSON.stringify([ - req.body.id, - req.body.color, - req.body.size, - req.body.owner, - req.body.appraisedValue, - ]); + const assetId = req.body.id; try { - const timestamp = Date.now(); - - // Store the transaction details and set the event handler in case there - // are problems later with commiting the transaction - await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp); - txn.setEventHandler(createDeferredEventHandler(redis)); - - await txn.submit( - req.body.id, + await submitTransaction( + contract, + redis, + 'CreateAsset', + assetId, req.body.color, req.body.size, req.body.owner, @@ -101,25 +92,22 @@ assetsRouter.post( timestamp: new Date().toISOString(), }); } catch (err) { - // TODO will this always catch endorsement errors or can those - // arrive later? - - // There's no point retrying a transaction if there were business - // logic errors so clear the transaction details - // - // Note: it would be nice to pick out business logic errors returned - // from chaincode, e.g. asset already exists, and return those as a - // 400 error with message instead. Unfortunately the asset transfer - // sample or Fabric Node SDK do not provide any well defined error - // codes that can be checked. - await clearTransactionDetails(redis, txnId); - logger.error( err, 'Error processing create asset request for asset ID %s with transaction ID %s', - req.body.id, - txnId + assetId, + err.transactionId ); + + if (err instanceof AssetExistsError) { + return res.status(CONFLICT).json({ + status: getReasonPhrase(CONFLICT), + reason: 'ASSET_EXISTS', + message: err.message, + timestamp: new Date().toISOString(), + }); + } + return res.status(INTERNAL_SERVER_ERROR).json({ status: getReasonPhrase(INTERNAL_SERVER_ERROR), timestamp: new Date().toISOString(), @@ -135,7 +123,7 @@ assetsRouter.options('/:assetId', async (req: Request, res: Response) => { try { const contract: Contract = req.app.get('contract'); - const data = await contract.evaluateTransaction('AssetExists', assetId); + const data = await evatuateTransaction(contract, 'AssetExists', assetId); const exists = data.toString() === 'true'; if (exists) { @@ -174,7 +162,7 @@ assetsRouter.get('/:assetId', async (req: Request, res: Response) => { try { const contract: Contract = req.app.get('contract'); - const data = await contract.evaluateTransaction('ReadAsset', assetId); + const data = await evatuateTransaction(contract, 'ReadAsset', assetId); const asset = JSON.parse(data.toString()); return res.status(OK).json(asset); @@ -184,6 +172,14 @@ assetsRouter.get('/:assetId', async (req: Request, res: Response) => { 'Error processing read asset request for asset ID %s', assetId ); + + if (err instanceof AssetNotFoundError) { + return res.status(NOT_FOUND).json({ + status: getReasonPhrase(NOT_FOUND), + timestamp: new Date().toISOString(), + }); + } + return res.status(INTERNAL_SERVER_ERROR).json({ status: getReasonPhrase(INTERNAL_SERVER_ERROR), timestamp: new Date().toISOString(), @@ -191,7 +187,6 @@ assetsRouter.get('/:assetId', async (req: Request, res: Response) => { } }); -// TODO this shares a lot of code with the post endpoint! assetsRouter.put( '/:assetId', body().isObject().withMessage('body must contain an asset object'), @@ -207,6 +202,7 @@ assetsRouter.put( if (!errors.isEmpty()) { return res.status(BAD_REQUEST).json({ status: getReasonPhrase(BAD_REQUEST), + reason: 'VALIDATION_ERROR', message: 'Invalid request body', timestamp: new Date().toISOString(), errors: errors.array(), @@ -216,6 +212,7 @@ assetsRouter.put( if (req.params.assetId != req.body.id) { return res.status(BAD_REQUEST).json({ status: getReasonPhrase(BAD_REQUEST), + reason: 'ASSET_ID_MISMATCH', message: 'Asset IDs must match', timestamp: new Date().toISOString(), }); @@ -223,27 +220,14 @@ assetsRouter.put( const contract: Contract = req.app.get('contract'); const redis: Redis = req.app.get('redis'); - const txn = contract.createTransaction('UpdateAsset'); - const txnId = txn.getTransactionId(); - const txnState = txn.serialize(); - const txnArgs = JSON.stringify([ - req.params.assetId, - req.body.color, - req.body.size, - req.body.owner, - req.body.appraisedValue, - ]); + const assetId = req.params.assetId; try { - const timestamp = Date.now(); - - // Store the transaction details and set the event handler in case there - // are problems later with commiting the transaction - await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp); - txn.setEventHandler(createDeferredEventHandler(redis)); - - await txn.submit( - req.params.assetId, + await submitTransaction( + contract, + redis, + 'UpdateAsset', + assetId, req.body.color, req.body.size, req.body.owner, @@ -255,25 +239,20 @@ assetsRouter.put( timestamp: new Date().toISOString(), }); } catch (err) { - // TODO will this always catch endorsement errors or can those - // arrive later? - - // There's no point retrying a transaction if there were business - // logic errors so clear the transaction details - // - // Note: it would be nice to pick out business logic errors returned - // from chaincode, e.g. asset already exists, and return those as a - // 400 error with message instead. Unfortunately the asset transfer - // sample or Fabric Node SDK do not provide any well defined error - // codes that can be checked. - await clearTransactionDetails(redis, txnId); - logger.error( err, 'Error processing update asset request for asset ID %s with transaction ID %s', - req.params.assetId, - txnId + assetId, + err.transactionId ); + + if (err instanceof AssetNotFoundError) { + return res.status(NOT_FOUND).json({ + status: getReasonPhrase(NOT_FOUND), + timestamp: new Date().toISOString(), + }); + } + return res.status(INTERNAL_SERVER_ERROR).json({ status: getReasonPhrase(INTERNAL_SERVER_ERROR), timestamp: new Date().toISOString(), @@ -282,7 +261,6 @@ assetsRouter.put( } ); -// TODO this shares a lot of code with the post endpoint! assetsRouter.patch( '/:assetId', body() @@ -301,56 +279,46 @@ assetsRouter.patch( if (!errors.isEmpty()) { return res.status(BAD_REQUEST).json({ status: getReasonPhrase(BAD_REQUEST), + reason: 'VALIDATION_ERROR', message: 'Invalid request body', timestamp: new Date().toISOString(), errors: errors.array(), }); } + const contract: Contract = req.app.get('contract'); + const redis: Redis = req.app.get('redis'); const assetId = req.params.assetId; const newOwner = req.body[0].value; - const contract: Contract = req.app.get('contract'); - const redis: Redis = req.app.get('redis'); - const txn = contract.createTransaction('TransferAsset'); - const txnId = txn.getTransactionId(); - const txnState = txn.serialize(); - const txnArgs = JSON.stringify([assetId, newOwner]); - try { - const timestamp = Date.now(); - - // Store the transaction details and set the event handler in case there - // are problems later with commiting the transaction - await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp); - txn.setEventHandler(createDeferredEventHandler(redis)); - - await txn.submit(assetId, newOwner); + await submitTransaction( + contract, + redis, + 'TransferAsset', + assetId, + newOwner + ); return res.status(ACCEPTED).json({ status: getReasonPhrase(ACCEPTED), timestamp: new Date().toISOString(), }); } catch (err) { - // TODO will this always catch endorsement errors or can those - // arrive later? - - // There's no point retrying a transaction if there were business - // logic errors so clear the transaction details - // - // Note: it would be nice to pick out business logic errors returned - // from chaincode, e.g. asset already exists, and return those as a - // 400 error with message instead. Unfortunately the asset transfer - // sample or Fabric Node SDK do not provide any well defined error - // codes that can be checked. - await clearTransactionDetails(redis, txnId); - logger.error( err, 'Error processing update asset request for asset ID %s with transaction ID %s', req.params.assetId, - txnId + err.transactionId ); + + if (err instanceof AssetNotFoundError) { + return res.status(NOT_FOUND).json({ + status: getReasonPhrase(NOT_FOUND), + timestamp: new Date().toISOString(), + }); + } + return res.status(INTERNAL_SERVER_ERROR).json({ status: getReasonPhrase(INTERNAL_SERVER_ERROR), timestamp: new Date().toISOString(), @@ -364,45 +332,30 @@ assetsRouter.delete('/:assetId', async (req: Request, res: Response) => { const contract: Contract = req.app.get('contract'); const redis: Redis = req.app.get('redis'); - const txn = contract.createTransaction('DeleteAsset'); - const txnId = txn.getTransactionId(); - const txnState = txn.serialize(); - const txnArgs = JSON.stringify([req.params.assetId]); + const assetId = req.params.assetId; try { - const timestamp = Date.now(); - - // Store the transaction details and set the event handler in case there - // are problems later with commiting the transaction - await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp); - txn.setEventHandler(createDeferredEventHandler(redis)); - - await txn.submit(req.params.assetId); + await submitTransaction(contract, redis, 'DeleteAsset', assetId); return res.status(ACCEPTED).json({ status: getReasonPhrase(ACCEPTED), timestamp: new Date().toISOString(), }); } catch (err) { - // TODO will this always catch endorsement errors or can those - // arrive later? - - // There's no point retrying a transaction if there were business - // logic errors so clear the transaction details - // - // Note: it would be nice to pick out business logic errors returned - // from chaincode, e.g. asset already exists, and return those as a - // 400 error with message instead. Unfortunately the asset transfer - // sample or Fabric Node SDK do not provide any well defined error - // codes that can be checked. - await clearTransactionDetails(redis, txnId); - logger.error( err, 'Error processing delete asset request for asset ID %s with transaction ID %s', - req.params.assetId, - txnId + assetId, + err.transactionId ); + + if (err instanceof AssetNotFoundError) { + return res.status(NOT_FOUND).json({ + status: getReasonPhrase(NOT_FOUND), + timestamp: new Date().toISOString(), + }); + } + return res.status(INTERNAL_SERVER_ERROR).json({ status: getReasonPhrase(INTERNAL_SERVER_ERROR), timestamp: new Date().toISOString(), diff --git a/asset-transfer-basic/rest-api-typescript/src/errors.ts b/asset-transfer-basic/rest-api-typescript/src/errors.ts new file mode 100644 index 00000000..beb03478 --- /dev/null +++ b/asset-transfer-basic/rest-api-typescript/src/errors.ts @@ -0,0 +1,33 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + */ + +export class TransactionError extends Error { + transactionId: string; + + constructor(message: string, transactionId: string) { + super(message); + Object.setPrototypeOf(this, TransactionError.prototype); + + this.name = 'TransactionError'; + this.transactionId = transactionId; + } +} + +export class AssetExistsError extends TransactionError { + constructor(message: string, transactionId: string) { + super(message, transactionId); + Object.setPrototypeOf(this, AssetExistsError.prototype); + + this.name = 'AssetExistsError'; + } +} + +export class AssetNotFoundError extends TransactionError { + constructor(message: string, transactionId: string) { + super(message, transactionId); + Object.setPrototypeOf(this, AssetNotFoundError.prototype); + + this.name = 'AssetNotFoundError'; + } +} diff --git a/asset-transfer-basic/rest-api-typescript/src/fabric.ts b/asset-transfer-basic/rest-api-typescript/src/fabric.ts index 495238f3..cb4e551a 100644 --- a/asset-transfer-basic/rest-api-typescript/src/fabric.ts +++ b/asset-transfer-basic/rest-api-typescript/src/fabric.ts @@ -16,6 +16,12 @@ import { import { Redis } from 'ioredis'; import * as config from './config'; import { logger } from './logger'; +import { storeTransactionDetails, clearTransactionDetails } from './redis'; +import { + AssetExistsError, + AssetNotFoundError, + TransactionError, +} from './errors'; export const getContract = async (): Promise => { const wallet = await Wallets.newInMemoryWallet(); @@ -174,6 +180,86 @@ export const startRetryLoop = (contract: Contract, redis: Redis): void => { ); }; +export const evatuateTransaction = async ( + contract: Contract, + transactionName: string, + ...transactionArgs: string[] +): Promise => { + const txn = contract.createTransaction(transactionName); + const txnId = txn.getTransactionId(); + + try { + return await txn.evaluate(...transactionArgs); + } catch (err) { + throw handleError(txnId, err); + } +}; + +export const submitTransaction = async ( + contract: Contract, + redis: Redis, + transactionName: string, + ...transactionArgs: string[] +): Promise => { + const txn = contract.createTransaction(transactionName); + const txnId = txn.getTransactionId(); + const txnState = txn.serialize(); + const txnArgs = JSON.stringify(transactionArgs); + const timestamp = Date.now(); + + try { + // Store the transaction details and set the event handler in case there + // are problems later with commiting the transaction + await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp); + txn.setEventHandler(createDeferredEventHandler(redis)); + + await txn.submit(...transactionArgs); + } catch (err) { + // If the transaction failed to endorse, there is no point attempting + // to retry it later so clear the transaction details + // TODO will this always catch endorsement errors or can they + // arrive later? + await clearTransactionDetails(redis, txnId); + throw handleError(txnId, err); + } + + return txnId; +}; + +// Unfortunately the chaincode samples do not use error codes, and the error +// message text is not the same for each implementation +const handleError = (transactionId: string, err: Error): Error => { + // This regex needs to match the following error messages: + // "the asset %s already exists" + // "The asset ${id} already exists" + // "Asset %s already exists" + const assetAlreadyExistsRegex = /([tT]he )?[aA]sset \w* already exists/g; + const assetAlreadyExistsMatch = err.message.match(assetAlreadyExistsRegex); + logger.debug( + { message: err.message, result: assetAlreadyExistsMatch }, + 'Checking for asset already exists message' + ); + if (assetAlreadyExistsMatch) { + return new AssetExistsError(assetAlreadyExistsMatch[0], transactionId); + } + + // This regex needs to match the following error messages: + // "the asset %s does not exist" + // "The asset ${id} does not exist" + // "Asset %s does not exist" + const assetDoesNotExistRegex = /([tT]he )?[aA]sset \w* does not exist/g; + const assetDoesNotExistMatch = err.message.match(assetDoesNotExistRegex); + logger.debug( + { message: err.message, result: assetDoesNotExistMatch }, + 'Checking for asset does not exist message' + ); + if (assetDoesNotExistMatch) { + return new AssetNotFoundError(assetDoesNotExistMatch[0], transactionId); + } + + return new TransactionError('Transaction error', transactionId); +}; + const retryTransaction = async ( contract: Contract, redis: Redis, @@ -225,58 +311,3 @@ const isDuplicateTransaction = (error: { return false; }; - -// TODO move these to redis.ts? - -export const storeTransactionDetails = async ( - redis: Redis, - transactionId: string, - transactionState: Buffer, - transactionArgs: string, - timestamp: number -): Promise => { - const key = `txn:${transactionId}`; - logger.debug( - 'Storing transaction details. Key: %s State: %s Args: %s Timestamp: %d', - key, - transactionState, - transactionArgs, - timestamp - ); - await redis - .multi() - .hset( - key, - 'state', - transactionState, - 'args', - transactionArgs, - 'timestamp', - timestamp, - 'retries', - '0' - ) - .zadd('index:txn:timestamp', timestamp, transactionId) - .exec(); -}; - -export const clearTransactionDetails = async ( - redis: Redis, - transactionId: string -): Promise => { - const key = `txn:${transactionId}`; - logger.debug('Removing transaction details. Key: %s', key); - try { - await redis - .multi() - .del(key) - .zrem('index:txn:timestamp', transactionId) - .exec(); - } catch (err) { - logger.error( - err, - 'Error remove saved transaction state for transaction ID %s', - transactionId - ); - } -}; diff --git a/asset-transfer-basic/rest-api-typescript/src/redis.ts b/asset-transfer-basic/rest-api-typescript/src/redis.ts index 9cc79dba..d799d800 100644 --- a/asset-transfer-basic/rest-api-typescript/src/redis.ts +++ b/asset-transfer-basic/rest-api-typescript/src/redis.ts @@ -2,9 +2,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import IORedis, { RedisOptions } from 'ioredis'; +import IORedis, { Redis, RedisOptions } from 'ioredis'; import * as config from './config'; +import { logger } from './logger'; const redisOptions: RedisOptions = { port: config.redisPort, @@ -14,3 +15,58 @@ const redisOptions: RedisOptions = { }; export const redis = new IORedis(redisOptions); + +export const storeTransactionDetails = async ( + redis: Redis, + transactionId: string, + transactionState: Buffer, + transactionArgs: string, + timestamp: number +): Promise => { + const key = `txn:${transactionId}`; + logger.debug( + 'Storing transaction details. Key: %s State: %s Args: %s Timestamp: %d', + key, + transactionState, + transactionArgs, + timestamp + ); + await redis + .multi() + .hset( + key, + 'state', + transactionState, + 'args', + transactionArgs, + 'timestamp', + timestamp, + 'retries', + '0' + ) + .zadd('index:txn:timestamp', timestamp, transactionId) + .exec(); +}; + +export const clearTransactionDetails = async ( + redis: Redis, + transactionId: string +): Promise => { + const key = `txn:${transactionId}`; + logger.debug('Removing transaction details. Key: %s', key); + try { + await redis + .multi() + .del(key) + .zrem('index:txn:timestamp', transactionId) + .exec(); + } catch (err) { + logger.error( + err, + 'Error remove saved transaction state for transaction ID %s', + transactionId + ); + } +}; + +// TODO add getTransaction etc. helpers?