Skip to content

Commit 3041201

Browse files
cpatino-intivecpatino-intive
committed
feat(awm): fix feedback
1 parent ca36090 commit 3041201

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
@@ -229,29 +229,6 @@ export class AdvancedWalletManagerClient {
229229
});
230230
}
231231

232-
private checkServerRunning(error: any): void {
233-
// Check for specific connection errors
234-
if (
235-
error.code === 'ECONNREFUSED' ||
236-
error.code === 'ENOTFOUND' ||
237-
error.code === 'ECONNRESET'
238-
) {
239-
throw new Error(
240-
'Advanced Wallet Manager API is not running or unreachable. Please check if the service is available.',
241-
);
242-
}
243-
if (error.code === 'ETIMEDOUT' || error.timeout) {
244-
throw new Error(
245-
'Advanced Wallet Manager API request timed out. The service may be overloaded or unreachable.',
246-
);
247-
}
248-
if (error.status === 404) {
249-
throw new Error(
250-
'Advanced Wallet Manager API endpoint not found. Please verify the URL configuration.',
251-
);
252-
}
253-
}
254-
255232
async recoveryMPC(params: {
256233
unsignedSweepPrebuildTx: MPCTx | MPCSweepTxs | MPCTxs | RecoveryTxRequest;
257234
userPub: string;
@@ -292,8 +269,6 @@ export class AdvancedWalletManagerClient {
292269
const response = await request.decodeExpecting(200);
293270
return response.body;
294271
} catch (error) {
295-
this.checkServerRunning(error);
296-
297272
const err = error as Error;
298273
logger.error('Failed to recover MPC: %s', err.message);
299274
throw err;
@@ -325,11 +300,9 @@ export class AdvancedWalletManagerClient {
325300
const response = await request.decodeExpecting(200);
326301
return response.body;
327302
} catch (error) {
328-
this.checkServerRunning(error);
329-
330303
logger.error(
331304
'Failed to create independent keychain: %s',
332-
(error as DecodeError).decodedResponse.body,
305+
(error as DecodeError).decodedResponse?.body,
333306
);
334307
throw error;
335308
}
@@ -359,9 +332,7 @@ export class AdvancedWalletManagerClient {
359332

360333
return response.body;
361334
} catch (error) {
362-
this.checkServerRunning(error);
363-
364-
logger.error('Failed to sign multisig: %s', (error as DecodeError).decodedResponse.body);
335+
logger.error('Failed to sign multisig: %s', (error as DecodeError).decodedResponse?.body);
365336
throw error;
366337
}
367338
}
@@ -384,11 +355,9 @@ export class AdvancedWalletManagerClient {
384355
logger.info('Enclaved express service ping successful');
385356
return response.body;
386357
} catch (error) {
387-
this.checkServerRunning(error);
388-
389358
logger.error(
390359
'Failed to ping enclaved express service: %s',
391-
(error as DecodeError).decodedResponse.body,
360+
(error as DecodeError).decodedResponse?.body,
392361
);
393362
throw error;
394363
}
@@ -411,11 +380,9 @@ export class AdvancedWalletManagerClient {
411380
logger.info('Successfully retrieved version information');
412381
return response.body;
413382
} catch (error) {
414-
this.checkServerRunning(error);
415-
416383
logger.error(
417384
'Failed to get version information: %s',
418-
(error as DecodeError).decodedResponse.body,
385+
(error as DecodeError).decodedResponse?.body,
419386
);
420387
throw error;
421388
}
@@ -440,9 +407,7 @@ export class AdvancedWalletManagerClient {
440407

441408
return res.body;
442409
} catch (error) {
443-
this.checkServerRunning(error);
444-
445-
logger.error('Failed to recover multisig: %s', (error as DecodeError).decodedResponse.body);
410+
logger.error('Failed to recover multisig: %s', (error as DecodeError).decodedResponse?.body);
446411
throw error;
447412
}
448413
}
@@ -473,11 +438,9 @@ export class AdvancedWalletManagerClient {
473438
const response = await request.decodeExpecting(200);
474439
return response.body;
475440
} catch (error) {
476-
this.checkServerRunning(error);
477-
478441
logger.error(
479442
'Failed to initialize MPC key generation: %s',
480-
(error as DecodeError).decodedResponse.body,
443+
(error as DecodeError).decodedResponse?.body,
481444
);
482445
throw error;
483446
}
@@ -523,11 +486,9 @@ export class AdvancedWalletManagerClient {
523486
const response = await request.decodeExpecting(200);
524487
return response.body;
525488
} catch (error) {
526-
this.checkServerRunning(error);
527-
528489
logger.error(
529490
'Failed to finalize MPC key generation: %s',
530-
(error as DecodeError).decodedResponse.body,
491+
(error as DecodeError).decodedResponse?.body,
531492
);
532493
throw error;
533494
}
@@ -551,11 +512,9 @@ export class AdvancedWalletManagerClient {
551512
const response = await request.decodeExpecting(200);
552513
return response.body;
553514
} catch (error) {
554-
this.checkServerRunning(error);
555-
556515
logger.error(
557516
'Failed to sign mpc commitment: %s',
558-
(error as DecodeError).decodedResponse.body,
517+
(error as DecodeError).decodedResponse?.body,
559518
);
560519
throw error;
561520
}
@@ -579,9 +538,7 @@ export class AdvancedWalletManagerClient {
579538
const response = await request.decodeExpecting(200);
580539
return response.body;
581540
} catch (error) {
582-
this.checkServerRunning(error);
583-
584-
logger.error('Failed to sign mpc r-share: %s', (error as DecodeError).decodedResponse.body);
541+
logger.error('Failed to sign mpc r-share: %s', (error as DecodeError).decodedResponse?.body);
585542
throw error;
586543
}
587544
}
@@ -604,9 +561,7 @@ export class AdvancedWalletManagerClient {
604561
const response = await request.decodeExpecting(200);
605562
return response.body;
606563
} catch (error) {
607-
this.checkServerRunning(error);
608-
609-
logger.error('Failed to sign mpc g-share: %s', (error as DecodeError).decodedResponse.body);
564+
logger.error('Failed to sign mpc g-share: %s', (error as DecodeError).decodedResponse?.body);
610565
throw error;
611566
}
612567
}
@@ -635,11 +590,9 @@ export class AdvancedWalletManagerClient {
635590
const response = await request.decodeExpecting(200);
636591
return response.body;
637592
} catch (error) {
638-
this.checkServerRunning(error);
639-
640593
logger.error(
641594
'Failed to initialize MPCv2 key generation: %s',
642-
(error as DecodeError).decodedResponse.body,
595+
(error as DecodeError).decodedResponse?.body,
643596
);
644597
throw error;
645598
}
@@ -676,11 +629,9 @@ export class AdvancedWalletManagerClient {
676629
const response = await request.decodeExpecting(200);
677630
return response.body;
678631
} catch (error) {
679-
this.checkServerRunning(error);
680-
681632
logger.error(
682633
'Failed to execute MPCv2 round: %s',
683-
(error as DecodeError).decodedResponse.body,
634+
(error as DecodeError).decodedResponse?.body,
684635
);
685636
throw error;
686637
}
@@ -714,11 +665,9 @@ export class AdvancedWalletManagerClient {
714665
const response = await request.decodeExpecting(200);
715666
return response.body;
716667
} catch (error) {
717-
this.checkServerRunning(error);
718-
719668
logger.error(
720669
'Failed to finalize MPCv2 key generation: %s',
721-
(error as DecodeError).decodedResponse.body,
670+
(error as DecodeError).decodedResponse?.body,
722671
);
723672
throw error;
724673
}
@@ -751,9 +700,10 @@ export class AdvancedWalletManagerClient {
751700
const response = await request.decodeExpecting(200);
752701
return response.body;
753702
} catch (error) {
754-
this.checkServerRunning(error);
755-
756-
logger.error('Failed to sign MPCv2 round 1: %s', (error as DecodeError).decodedResponse.body);
703+
logger.error(
704+
'Failed to sign MPCv2 round 1: %s',
705+
(error as DecodeError).decodedResponse?.body,
706+
);
757707
throw error;
758708
}
759709
}
@@ -785,9 +735,10 @@ export class AdvancedWalletManagerClient {
785735
const response = await request.decodeExpecting(200);
786736
return response.body;
787737
} catch (error) {
788-
this.checkServerRunning(error);
789-
790-
logger.error('Failed to sign MPCv2 round 2: %s', (error as DecodeError).decodedResponse.body);
738+
logger.error(
739+
'Failed to sign MPCv2 round 2: %s',
740+
(error as DecodeError).decodedResponse?.body,
741+
);
791742
throw error;
792743
}
793744
}
@@ -819,9 +770,10 @@ export class AdvancedWalletManagerClient {
819770
const response = await request.decodeExpecting(200);
820771
return response.body;
821772
} catch (error) {
822-
this.checkServerRunning(error);
823-
824-
logger.error('Failed to sign MPCv2 round 3: %s', (error as DecodeError).decodedResponse.body);
773+
logger.error(
774+
'Failed to sign MPCv2 round 3: %s',
775+
(error as DecodeError).decodedResponse?.body,
776+
);
825777
throw error;
826778
}
827779
}
@@ -848,11 +800,9 @@ export class AdvancedWalletManagerClient {
848800
const response = await request.decodeExpecting(200);
849801
return response.body;
850802
} catch (error: any) {
851-
this.checkServerRunning(error);
852-
853803
logger.error(
854804
'Failed to recover MPCv2 wallet: %s',
855-
(error as DecodeError).decodedResponse.body,
805+
(error as DecodeError).decodedResponse?.body,
856806
);
857807
throw error;
858808
}

‎src/kms/kmsClient.ts‎

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

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

5961
switch (error.status) {
6062
case 400:
@@ -74,22 +76,6 @@ export class KmsClient {
7476
}
7577
}
7678

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

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

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

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

204184
// 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)