Skip to content

Commit 7b6dab1

Browse files
cpatino-intivecpatino-intive
committed
feat(awm): fix feedback
1 parent 6e1bbff commit 7b6dab1

File tree

3 files changed

+69
-100
lines changed

3 files changed

+69
-100
lines changed

‎src/api/master/clients/advancedWalletManagerClient.ts‎

Lines changed: 26 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -231,29 +231,6 @@ export class AdvancedWalletManagerClient {
231231
});
232232
}
233233

234-
private checkServerRunning(error: any): void {
235-
// Check for specific connection errors
236-
if (
237-
error.code === 'ECONNREFUSED' ||
238-
error.code === 'ENOTFOUND' ||
239-
error.code === 'ECONNRESET'
240-
) {
241-
throw new Error(
242-
'Advanced Wallet Manager API is not running or unreachable. Please check if the service is available.',
243-
);
244-
}
245-
if (error.code === 'ETIMEDOUT' || error.timeout) {
246-
throw new Error(
247-
'Advanced Wallet Manager API request timed out. The service may be overloaded or unreachable.',
248-
);
249-
}
250-
if (error.status === 404) {
251-
throw new Error(
252-
'Advanced Wallet Manager API endpoint not found. Please verify the URL configuration.',
253-
);
254-
}
255-
}
256-
257234
async recoveryMPC(params: {
258235
unsignedSweepPrebuildTx: MPCTx | MPCSweepTxs | MPCTxs | RecoveryTxRequest;
259236
userPub: string;
@@ -294,8 +271,6 @@ export class AdvancedWalletManagerClient {
294271
const response = await request.decodeExpecting(200);
295272
return response.body;
296273
} catch (error) {
297-
this.checkServerRunning(error);
298-
299274
const err = error as Error;
300275
logger.error('Failed to recover MPC: %s', err.message);
301276
throw err;
@@ -327,11 +302,9 @@ export class AdvancedWalletManagerClient {
327302
const response = await request.decodeExpecting(200);
328303
return response.body;
329304
} catch (error) {
330-
this.checkServerRunning(error);
331-
332305
logger.error(
333306
'Failed to create independent keychain: %s',
334-
(error as DecodeError).decodedResponse.body,
307+
(error as DecodeError).decodedResponse?.body,
335308
);
336309
throw error;
337310
}
@@ -361,9 +334,7 @@ export class AdvancedWalletManagerClient {
361334

362335
return response.body;
363336
} catch (error) {
364-
this.checkServerRunning(error);
365-
366-
logger.error('Failed to sign multisig: %s', (error as DecodeError).decodedResponse.body);
337+
logger.error('Failed to sign multisig: %s', (error as DecodeError).decodedResponse?.body);
367338
throw error;
368339
}
369340
}
@@ -386,11 +357,9 @@ export class AdvancedWalletManagerClient {
386357
logger.info('Advanced Wallet Manager ping successful');
387358
return response.body;
388359
} catch (error) {
389-
this.checkServerRunning(error);
390-
391360
logger.error(
392361
'Failed to ping Advanced Wallet Manager: %s',
393-
(error as DecodeError).decodedResponse.body,
362+
(error as DecodeError).decodedResponse?.body,
394363
);
395364
throw error;
396365
}
@@ -413,11 +382,9 @@ export class AdvancedWalletManagerClient {
413382
logger.info('Successfully retrieved version information');
414383
return response.body;
415384
} catch (error) {
416-
this.checkServerRunning(error);
417-
418385
logger.error(
419386
'Failed to get version information: %s',
420-
(error as DecodeError).decodedResponse.body,
387+
(error as DecodeError).decodedResponse?.body,
421388
);
422389
throw error;
423390
}
@@ -442,9 +409,7 @@ export class AdvancedWalletManagerClient {
442409

443410
return res.body;
444411
} catch (error) {
445-
this.checkServerRunning(error);
446-
447-
logger.error('Failed to recover multisig: %s', (error as DecodeError).decodedResponse.body);
412+
logger.error('Failed to recover multisig: %s', (error as DecodeError).decodedResponse?.body);
448413
throw error;
449414
}
450415
}
@@ -475,11 +440,9 @@ export class AdvancedWalletManagerClient {
475440
const response = await request.decodeExpecting(200);
476441
return response.body;
477442
} catch (error) {
478-
this.checkServerRunning(error);
479-
480443
logger.error(
481444
'Failed to initialize MPC key generation: %s',
482-
(error as DecodeError).decodedResponse.body,
445+
(error as DecodeError).decodedResponse?.body,
483446
);
484447
throw error;
485448
}
@@ -525,11 +488,9 @@ export class AdvancedWalletManagerClient {
525488
const response = await request.decodeExpecting(200);
526489
return response.body;
527490
} catch (error) {
528-
this.checkServerRunning(error);
529-
530491
logger.error(
531492
'Failed to finalize MPC key generation: %s',
532-
(error as DecodeError).decodedResponse.body,
493+
(error as DecodeError).decodedResponse?.body,
533494
);
534495
throw error;
535496
}
@@ -553,11 +514,9 @@ export class AdvancedWalletManagerClient {
553514
const response = await request.decodeExpecting(200);
554515
return response.body;
555516
} catch (error) {
556-
this.checkServerRunning(error);
557-
558517
logger.error(
559518
'Failed to sign mpc commitment: %s',
560-
(error as DecodeError).decodedResponse.body,
519+
(error as DecodeError).decodedResponse?.body,
561520
);
562521
throw error;
563522
}
@@ -581,9 +540,7 @@ export class AdvancedWalletManagerClient {
581540
const response = await request.decodeExpecting(200);
582541
return response.body;
583542
} catch (error) {
584-
this.checkServerRunning(error);
585-
586-
logger.error('Failed to sign mpc r-share: %s', (error as DecodeError).decodedResponse.body);
543+
logger.error('Failed to sign mpc r-share: %s', (error as DecodeError).decodedResponse?.body);
587544
throw error;
588545
}
589546
}
@@ -606,9 +563,7 @@ export class AdvancedWalletManagerClient {
606563
const response = await request.decodeExpecting(200);
607564
return response.body;
608565
} catch (error) {
609-
this.checkServerRunning(error);
610-
611-
logger.error('Failed to sign mpc g-share: %s', (error as DecodeError).decodedResponse.body);
566+
logger.error('Failed to sign mpc g-share: %s', (error as DecodeError).decodedResponse?.body);
612567
throw error;
613568
}
614569
}
@@ -637,11 +592,9 @@ export class AdvancedWalletManagerClient {
637592
const response = await request.decodeExpecting(200);
638593
return response.body;
639594
} catch (error) {
640-
this.checkServerRunning(error);
641-
642595
logger.error(
643596
'Failed to initialize MPCv2 key generation: %s',
644-
(error as DecodeError).decodedResponse.body,
597+
(error as DecodeError).decodedResponse?.body,
645598
);
646599
throw error;
647600
}
@@ -678,11 +631,9 @@ export class AdvancedWalletManagerClient {
678631
const response = await request.decodeExpecting(200);
679632
return response.body;
680633
} catch (error) {
681-
this.checkServerRunning(error);
682-
683634
logger.error(
684635
'Failed to execute MPCv2 round: %s',
685-
(error as DecodeError).decodedResponse.body,
636+
(error as DecodeError).decodedResponse?.body,
686637
);
687638
throw error;
688639
}
@@ -716,11 +667,9 @@ export class AdvancedWalletManagerClient {
716667
const response = await request.decodeExpecting(200);
717668
return response.body;
718669
} catch (error) {
719-
this.checkServerRunning(error);
720-
721670
logger.error(
722671
'Failed to finalize MPCv2 key generation: %s',
723-
(error as DecodeError).decodedResponse.body,
672+
(error as DecodeError).decodedResponse?.body,
724673
);
725674
throw error;
726675
}
@@ -753,9 +702,10 @@ export class AdvancedWalletManagerClient {
753702
const response = await request.decodeExpecting(200);
754703
return response.body;
755704
} catch (error) {
756-
this.checkServerRunning(error);
757-
758-
logger.error('Failed to sign MPCv2 round 1: %s', (error as DecodeError).decodedResponse.body);
705+
logger.error(
706+
'Failed to sign MPCv2 round 1: %s',
707+
(error as DecodeError).decodedResponse?.body,
708+
);
759709
throw error;
760710
}
761711
}
@@ -787,9 +737,10 @@ export class AdvancedWalletManagerClient {
787737
const response = await request.decodeExpecting(200);
788738
return response.body;
789739
} catch (error) {
790-
this.checkServerRunning(error);
791-
792-
logger.error('Failed to sign MPCv2 round 2: %s', (error as DecodeError).decodedResponse.body);
740+
logger.error(
741+
'Failed to sign MPCv2 round 2: %s',
742+
(error as DecodeError).decodedResponse?.body,
743+
);
793744
throw error;
794745
}
795746
}
@@ -821,9 +772,10 @@ export class AdvancedWalletManagerClient {
821772
const response = await request.decodeExpecting(200);
822773
return response.body;
823774
} catch (error) {
824-
this.checkServerRunning(error);
825-
826-
logger.error('Failed to sign MPCv2 round 3: %s', (error as DecodeError).decodedResponse.body);
775+
logger.error(
776+
'Failed to sign MPCv2 round 3: %s',
777+
(error as DecodeError).decodedResponse?.body,
778+
);
827779
throw error;
828780
}
829781
}
@@ -850,11 +802,9 @@ export class AdvancedWalletManagerClient {
850802
const response = await request.decodeExpecting(200);
851803
return response.body;
852804
} catch (error: any) {
853-
this.checkServerRunning(error);
854-
855805
logger.error(
856806
'Failed to recover MPCv2 wallet: %s',
857-
(error as DecodeError).decodedResponse.body,
807+
(error as DecodeError).decodedResponse?.body,
858808
);
859809
throw error;
860810
}

‎src/kms/kmsClient.ts‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ export class KmsClient {
5555
private errorHandler(error: superagent.ResponseError, errorLog: string) {
5656
logger.error(errorLog, error);
5757

58-
this.checkServerRunning(error);
58+
if (['ECONNREFUSED', 'ENOTFOUND', 'ECONNRESET', 'ETIMEDOUT'].includes((error as any).code)) {
59+
throw error;
60+
}
5961

6062
switch (error.status) {
6163
case 400:
@@ -75,22 +77,6 @@ export class KmsClient {
7577
}
7678
}
7779

78-
private checkServerRunning(error: any): void {
79-
// Check for specific connection errors
80-
if (
81-
error.code === 'ECONNREFUSED' ||
82-
error.code === 'ENOTFOUND' ||
83-
error.code === 'ECONNRESET'
84-
) {
85-
throw new Error(
86-
'KMS API is not running or unreachable. Please check if the KMS service is available.',
87-
);
88-
}
89-
if (error.code === 'ETIMEDOUT' || error.timeout) {
90-
throw new Error('KMS API request timed out. The service may be overloaded or unreachable.');
91-
}
92-
}
93-
9480
async postKey(params: PostKeyParams): Promise<PostKeyResponse> {
9581
logger.info('Posting key to KMS with pub: %s and source: %s', params.pub, params.source);
9682

@@ -135,8 +121,6 @@ export class KmsClient {
135121
kmsResponse = await req;
136122
} catch (error: any) {
137123
this.errorHandler(error, 'Error getting key from KMS');
138-
139-
throw new Error(`Failed to get key from KMS: ${error.message}`);
140124
}
141125

142126
// validate the response
@@ -165,8 +149,6 @@ export class KmsClient {
165149
kmsResponse = await req;
166150
} catch (error: any) {
167151
this.errorHandler(error, 'Error generating data key from KMS');
168-
169-
throw new Error(`Failed to generate data key from KMS: ${error.message}`);
170152
}
171153

172154
// validate the response
@@ -198,8 +180,6 @@ export class KmsClient {
198180
kmsResponse = await req;
199181
} catch (error: any) {
200182
this.errorHandler(error, 'Error decrypting data key from KMS');
201-
202-
throw new Error(`Failed to decrypt data key from KMS: ${error.message}`);
203183
}
204184

205185
// validate the response

‎src/shared/responseHandler.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Request, Response as ExpressResponse, NextFunction } from 'express';
2-
import { Config } from '../shared/types';
2+
import { AppMode, Config } from '../shared/types';
33
import { BitGoRequest } from '../types/request';
44
import { ApiResponseError, AdvancedWalletManagerError } from '../errors';
55
import {
@@ -32,6 +32,30 @@ export type ServiceFunction<T extends Config = Config> = (
3232
next: NextFunction,
3333
) => Promise<ApiResponse> | ApiResponse;
3434

35+
/**
36+
* Check for specific API connection errors and throw appropriate messages
37+
*/
38+
function checkApiServerRunning(req: BitGoRequest, error: any): void {
39+
const config = req.config;
40+
const isMbe = config.appMode === AppMode.MASTER_EXPRESS;
41+
42+
if (error.code === 'ECONNREFUSED' || error.code === 'ENOTFOUND' || error.code === 'ECONNRESET') {
43+
throw new Error(
44+
`${
45+
isMbe ? 'Advanced Wallet Manager' : 'KMS'
46+
} API service is not running or unreachable. Please check if the service is available.`,
47+
);
48+
}
49+
50+
if (error.code === 'ETIMEDOUT' || error.timeout) {
51+
throw new Error(
52+
`${
53+
isMbe ? 'Advanced Wallet Manager' : 'KMS'
54+
} API request timed out. The service may be overloaded or unreachable.`,
55+
);
56+
}
57+
}
58+
3559
/**
3660
* Wraps a service function to handle Response objects and errors consistently
3761
* @param fn Service function that returns a Response object
@@ -43,6 +67,21 @@ export function responseHandler<T extends Config = Config>(fn: ServiceFunction<T
4367
const result = await fn(req as BitGoRequest<T>, res, next);
4468
return res.sendEncoded(result.type, result.payload);
4569
} catch (error) {
70+
// Check for API connection errors first, but don't throw if it's not a connection issue
71+
try {
72+
checkApiServerRunning(req as BitGoRequest, error);
73+
} catch (connectionError) {
74+
// If checkApiServerRunning throws, use that error message
75+
const errorBody = {
76+
error: 'Internal Server Error',
77+
name: 'ConnectionError',
78+
details:
79+
connectionError instanceof Error ? connectionError.message : String(connectionError),
80+
};
81+
logger.error('API Connection Error: %s', errorBody.details);
82+
return res.sendEncoded(500, errorBody);
83+
}
84+
4685
// If it's already a Response object (e.g. from Response.error)
4786
if (error && typeof error === 'object' && 'type' in error && 'payload' in error) {
4887
const apiError = error as ApiResponse;

0 commit comments

Comments
 (0)