diff --git a/src/__tests__/api/master/accelerate.test.ts b/src/__tests__/api/master/accelerate.test.ts index fdd80aae..a2ddebcc 100644 --- a/src/__tests__/api/master/accelerate.test.ts +++ b/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(); diff --git a/src/__tests__/api/master/consolidateUnspents.test.ts b/src/__tests__/api/master/consolidateUnspents.test.ts index 3665c53e..310437ab 100644 --- a/src/__tests__/api/master/consolidateUnspents.test.ts +++ b/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}`) diff --git a/src/__tests__/api/master/sendMany.test.ts b/src/__tests__/api/master/sendMany.test.ts index d6e273fd..370ebd5e 100644 --- a/src/__tests__/api/master/sendMany.test.ts +++ b/src/__tests__/api/master/sendMany.test.ts @@ -5,7 +5,7 @@ import * as request from 'supertest'; import nock from 'nock'; import { app as expressApp } from '../../../masterExpressApp'; import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; -import { Environments, Wallet } from '@bitgo/sdk-core'; +import { ApiResponseError, Environments, Wallet } from '@bitgo/sdk-core'; import { Coin } from 'bitgo'; import assert from 'assert'; @@ -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,11 +630,85 @@ describe('POST /api/:coin/wallet/:walletId/sendmany', () => { pubkey: 'xpub_user', }); + response.status.should.equal(400); + + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(prebuildStub); + sinon.assert.calledOnce(verifyStub); + }); + + it('should handle BitGoApiResponseError correctly', async () => { + // Mock wallet get request + 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'], + }); + + // Mock keychain get request + 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 prebuildStub = sinon.stub(Wallet.prototype, 'prebuildTransaction').resolves({ + txHex: 'prebuilt-tx-hex', + txInfo: { + nP2SHInputs: 1, + nSegwitInputs: 0, + nOutputs: 2, + }, + walletId, + }); + + const verifyStub = sinon.stub(Coin.Btc.prototype, 'verifyTransaction').resolves(true); + + // Mock enclaved express sign request to return an error + const signNock = nock(enclavedExpressUrl) + .post(`/api/${coin}/multisig/sign`) + .replyWithError( + new ApiResponseError('Custom API error', 500, { + error: 'Custom API error', + requestId: 'test-request-id', + }), + ); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/sendMany`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + recipients: [ + { + address: 'tb1qtest1', + amount: '100000', + }, + ], + source: 'user', + pubkey: 'xpub_user', + }); + + // The response should be a 500 error with the enclaved error details response.status.should.equal(500); + response.body.should.have.property('error'); + response.body.should.have.property('details'); + response.body.error.should.equal('BitGoApiResponseError'); + response.body.details.should.deepEqual({ + error: 'Custom API error', + requestId: 'test-request-id', + }); walletGetNock.done(); keychainGetNock.done(); sinon.assert.calledOnce(prebuildStub); sinon.assert.calledOnce(verifyStub); + signNock.done(); }); }); diff --git a/src/api/master/handlers/handleSendMany.ts b/src/api/master/handlers/handleSendMany.ts index b47e495b..56fd042c 100644 --- a/src/api/master/handlers/handleSendMany.ts +++ b/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, NotFoundError } 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,7 +67,7 @@ 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'>) { @@ -81,11 +82,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 +99,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 +116,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 +153,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 +191,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)}`); diff --git a/src/api/master/routers/masterApiSpec.ts b/src/api/master/routers/masterApiSpec.ts index 2451b973..534b5d69 100644 --- a/src/api/master/routers/masterApiSpec.ts +++ b/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, }; diff --git a/src/shared/errors.ts b/src/shared/errors.ts index 1381596d..1fb2245c 100644 --- a/src/shared/errors.ts +++ b/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 }; diff --git a/src/shared/responseHandler.ts b/src/shared/responseHandler.ts index d958d859..ec149989 100644 --- a/src/shared/responseHandler.ts +++ b/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(fn: ServiceFunction