Refactor transaction logic

Remove duplication and handle errors from the asset transfer smart contract

Signed-off-by: James Taylor <jamest@uk.ibm.com>

.gitignore

@@ -0,0 +1 @@
+local.http

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 {
+import { AssetExistsError, AssetNotFoundError } from './errors';
-  clearTransactionDetails,
+import { evatuateTransaction, submitTransaction } from './fabric';
-  createDeferredEventHandler,
-  storeTransactionDetails,
-} from './fabric';
 import { logger } from './logger';
 
-const { ACCEPTED, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_FOUND, OK } =
+const {
-  StatusCodes;
+  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 assetId = req.body.id;
-    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,
-    ]);
 
     try {
-      const timestamp = Date.now();
+      await submitTransaction(
-
+        contract,
-      // Store the transaction details and set the event handler in case there
+        redis,
-      // are problems later with commiting the transaction
+        'CreateAsset',
-      await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp);
+        assetId,
-      txn.setEventHandler(createDeferredEventHandler(redis));
-
-      await txn.submit(
-        req.body.id,
         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,
+        assetId,
-        txnId
+        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 assetId = req.params.assetId;
-    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,
-    ]);
 
     try {
-      const timestamp = Date.now();
+      await submitTransaction(
-
+        contract,
-      // Store the transaction details and set the event handler in case there
+        redis,
-      // are problems later with commiting the transaction
+        'UpdateAsset',
-      await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp);
+        assetId,
-      txn.setEventHandler(createDeferredEventHandler(redis));
-
-      await txn.submit(
-        req.params.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,
+        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(),
@@ -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();
+      await submitTransaction(
-
+        contract,
-      // Store the transaction details and set the event handler in case there
+        redis,
-      // are problems later with commiting the transaction
+        'TransferAsset',
-      await storeTransactionDetails(redis, txnId, txnState, txnArgs, timestamp);
+        assetId,
-      txn.setEventHandler(createDeferredEventHandler(redis));
+        newOwner
-
+      );
-      await txn.submit(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 assetId = req.params.assetId;
-  const txnId = txn.getTransactionId();
-  const txnState = txn.serialize();
-  const txnArgs = JSON.stringify([req.params.assetId]);
 
   try {
-    const timestamp = Date.now();
+    await submitTransaction(contract, redis, 'DeleteAsset', assetId);
-
-    // 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);
 
     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,
+      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(),

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';
+  }
+}

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<Contract> => {
   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<Buffer> => {
+  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<string> => {
+  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<void> => {
-  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<void> => {
-  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
-    );
-  }
-};

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<void> => {
+  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<void> => {
+  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?