chore(mbe): handle bitgo api error responses

Ticket: WP-5260

Author: pranavjain97

src/__tests__/api/master/accelerate.test.ts

@@ -157,7 +157,7 @@ describe('POST /api/:coin/wallet/:walletId/accelerate', () => {
         cpfpTxIds: ['b8a828b98dbf32d9fd1875cbace9640ceb8c82626716b4a64203fdc79bb46d26'],
       });
 
-    response.status.should.equal(500);
+    response.status.should.equal(404);
     response.body.should.have.property('error');
 
     walletGetNock.done();
@@ -190,7 +190,7 @@ describe('POST /api/:coin/wallet/:walletId/accelerate', () => {
         cpfpTxIds: ['b8a828b98dbf32d9fd1875cbace9640ceb8c82626716b4a64203fdc79bb46d26'],
       });
 
-    response.status.should.equal(500);
+    response.status.should.equal(404);
     response.body.should.have.property('error');
 
     walletGetNock.done();

src/__tests__/api/master/consolidateUnspents.test.ts

@@ -107,55 +107,6 @@ describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => {
     sinon.assert.calledOnce(consolidateUnspentsStub);
   });
 
-  it('should return error, name, and details on failure', async () => {
-    const walletGetNock = nock(bitgoApiUrl)
-      .get(`/api/v2/${coin}/wallet/${walletId}`)
-      .matchHeader('any', () => true)
-      .reply(200, {
-        id: walletId,
-        type: 'cold',
-        subType: 'onPrem',
-        keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
-      });
-
-    const keychainGetNock = nock(bitgoApiUrl)
-      .get(`/api/v2/${coin}/key/user-key-id`)
-      .matchHeader('any', () => true)
-      .reply(200, {
-        id: 'user-key-id',
-        pub: 'xpub_user',
-      });
-
-    const mockError = {
-      error: 'Internal Server Error',
-      name: 'ApiResponseError',
-      details:
-        'There are too few unspents that meet the given parameters to consolidate (1 available).',
-    };
-
-    const consolidateUnspentsStub = sinon
-      .stub(Wallet.prototype, 'consolidateUnspents')
-      .throws(Object.assign(new Error(mockError.details), mockError));
-
-    const response = await agent
-      .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`)
-      .set('Authorization', `Bearer ${accessToken}`)
-      .send({
-        source: 'user',
-        pubkey: 'xpub_user',
-        feeRate: 1000,
-      });
-
-    response.status.should.equal(500);
-    response.body.should.have.property('error', mockError.error);
-    response.body.should.have.property('name', mockError.name);
-    response.body.should.have.property('details', mockError.details);
-
-    walletGetNock.done();
-    keychainGetNock.done();
-    sinon.assert.calledOnce(consolidateUnspentsStub);
-  });
-
   it('should throw error when provided pubkey does not match wallet keychain', async () => {
     const walletGetNock = nock(bitgoApiUrl)
       .get(`/api/v2/${coin}/wallet/${walletId}`)

src/__tests__/api/master/sendMany.test.ts

@@ -447,7 +447,7 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => {
           pubkey: 'xpub_backup',
         });
 
-      response.status.should.equal(500);
+      response.status.should.equal(400);
       response.body.details.should.equal('Backup MPC signing not supported for sendMany');
 
       walletGetNock.done();
@@ -492,8 +492,7 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => {
         pubkey: 'wrong_pubkey',
       });
 
-    // TODO: Fix this to expect the error message when the middleware on MBE is fixed to handle errors
-    response.status.should.equal(500);
+    response.status.should.equal(400);
 
     walletGetNock.done();
     keychainGetNock.done();
@@ -573,7 +572,7 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => {
         pubkey: 'xpub_user',
       });
 
-    response.status.should.equal(500);
+    response.status.should.equal(400);
 
     walletGetNock.done();
     keychainGetNock.done();
@@ -631,7 +630,7 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => {
         pubkey: 'xpub_user',
       });
 
-    response.status.should.equal(500);
+    response.status.should.equal(400);
 
     walletGetNock.done();
     keychainGetNock.done();

src/api/master/handlers/handleSendMany.ts

@@ -13,6 +13,7 @@ import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec';
 import { createEcdsaMPCv2CustomSigners } from './ecdsaMPCv2';
 import { EnclavedExpressClient } from '../clients/enclavedExpressClient';
 import { createEddsaCustomSigningFunctions } from './eddsa';
+import {BadRequestError, ForbiddenError, NotFoundError, UnprocessableEntityResponse} from '../../../shared/errors';
 
 /**
  * Defines the structure for a single recipient in a send-many transaction.
@@ -41,7 +42,7 @@ function createMPCSendParamsWithCustomSigningFns(
   const mpcAlgorithm = coin.getMPCAlgorithm();
 
   if (!commonKeychain) {
-    throw new Error('Common keychain is required for MPC signing');
+    throw new BadRequestError('Common keychain is required for MPC signing');
   }
 
   if (mpcAlgorithm === 'ecdsa') {
@@ -66,10 +67,11 @@ function createMPCSendParamsWithCustomSigningFns(
     };
   }
 
-  throw new Error(`Unsupported MPC algorithm: ${mpcAlgorithm}`);
+  throw new BadRequestError(`Unsupported MPC algorithm: ${mpcAlgorithm}`);
 }
 
 export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.sendMany', 'post'>) {
+
   const enclavedExpressClient = req.enclavedExpressClient;
   const reqId = new RequestTracer();
   const bitgo = req.bitgo;
@@ -81,11 +83,11 @@ export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.s
   const walletId = req.params.walletId;
   const wallet = await baseCoin.wallets().get({ id: walletId, reqId });
   if (!wallet) {
-    throw new Error(`Wallet ${walletId} not found`);
+    throw new NotFoundError(`Wallet ${walletId} not found`);
   }
 
   if (wallet.type() !== 'cold' || wallet.subType() !== 'onPrem') {
-    throw new Error('Wallet is not an on-prem wallet');
+    throw new NotFoundError('Wallet is not an on-prem wallet');
   }
 
   const keyIdIndex = params.source === 'user' ? KeyIndices.USER : KeyIndices.BACKUP;
@@ -98,13 +100,15 @@ export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.s
   });
 
   if (!signingKeychain) {
-    throw new Error(`Signing keychain for ${params.source} not found`);
+    throw new NotFoundError(`Signing keychain for ${params.source} not found`);
   }
   if (params.pubkey && signingKeychain.pub !== params.pubkey) {
-    throw new Error(`Pub provided does not match the keychain on wallet for ${params.source}`);
+    throw new BadRequestError(
+      `Pub provided does not match the keychain on wallet for ${params.source}`,
+    );
   }
   if (params.commonKeychain && signingKeychain.commonKeychain !== params.commonKeychain) {
-    throw new Error(
+    throw new BadRequestError(
       `Common keychain provided does not match the keychain on wallet for ${params.source}`,
     );
   }
@@ -113,7 +117,7 @@ export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.s
     // Create MPC send parameters with custom signing functions
     if (wallet.multisigType() === 'tss') {
       if (signingKeychain.source === 'backup') {
-        throw new Error('Backup MPC signing not supported for sendMany');
+        throw new BadRequestError('Backup MPC signing not supported for sendMany');
       }
       const mpcSendParams = createMPCSendParamsWithCustomSigningFns(
         req,
@@ -150,14 +154,14 @@ export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.s
         walletType: wallet.multisigType(),
       });
       if (!verified) {
-        throw new Error('Transaction prebuild failed local validation');
+        throw new BadRequestError('Transaction prebuild failed local validation');
       }
       logger.debug('Transaction prebuild verified');
     } catch (e) {
       const err = e as Error;
       logger.error('transaction prebuild failed local validation:', err.message);
       logger.error('transaction prebuild:', JSON.stringify(txPrebuilt, null, 2));
-      throw new Error(`Transaction prebuild failed local validation: ${err.message}`);
+      throw new BadRequestError(`Transaction prebuild failed local validation: ${err.message}`);
     }
 
     logger.debug('Tx prebuild: %s', JSON.stringify(txPrebuilt, null, 2));
@@ -188,7 +192,7 @@ export async function signAndSendMultisig(
   reqId: RequestTracer,
 ) {
   if (!signingKeychain.pub) {
-    throw new Error(`Signing keychain pub not found for ${source}`);
+    throw new BadRequestError(`Signing keychain pub not found for ${source}`);
   }
   logger.info(`Signing with ${source} keychain, pub: ${signingKeychain.pub}`);
   logger.debug(`Signing keychain: ${JSON.stringify(signingKeychain, null, 2)}`);

src/api/master/routers/masterApiSpec.ts

@@ -31,6 +31,7 @@ import {
   BadRequestResponse,
   InternalServerErrorResponse,
   UnprocessableEntityResponse,
+    NotFoundResponse,
 } from '../../../shared/errors';
 
 export type ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3;
@@ -158,6 +159,8 @@ export const SendManyRequest = {
 export const SendManyResponse: HttpResponse = {
   // TODO: Get type from public types repo / Wallet Platform
   200: t.any,
+  ...BadRequestResponse,
+  ...NotFoundResponse,
   ...InternalServerErrorResponse,
 };
 

src/shared/errors.ts

@@ -84,3 +84,4 @@ const ErrorResponse = t.type({
 export const BadRequestResponse = { 400: ErrorResponse };
 export const UnprocessableEntityResponse = { 422: ErrorResponse };
 export const InternalServerErrorResponse = { 500: ErrorResponse };
+export const NotFoundResponse = { 404: ErrorResponse };

src/shared/responseHandler.ts

@@ -1,7 +1,7 @@
 import { Request, Response as ExpressResponse, NextFunction } from 'express';
 import { Config } from '../shared/types';
 import { BitGoRequest } from '../types/request';
-import { EnclavedError } from '../errors';
+import { ApiResponseError, EnclavedError } from '../errors';
 import {
   BitgoExpressError,
   ValidationError,
@@ -47,6 +47,15 @@ export function responseHandler<T extends Config = Config>(fn: ServiceFunction<T
         return res.sendEncoded(apiError.type, apiError.payload);
       }
 
+      if ((error as any).name === 'ApiResponseError') {
+        const apiError = error as ApiResponseError;
+        const body = {
+          error: 'BitGoApiResponseError',
+          details: apiError.result,
+        };
+        return res.status(apiError.status).json(body);
+      }
+
       // If it's a BitgoExpressError, map to appropriate status code
       if (error instanceof BitgoExpressError) {
         let statusCode = 500;