From ce4769b4f25bcadeabacc336c75da1177f15c325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ant=C3=B3nio=20Regadas?= Date: Tue, 7 Nov 2023 14:48:34 +0000 Subject: [PATCH] chore(error logging): error logging was improved (#532) * chore(error logging): error logging was improved * fix(wording): improve message wording * fix(test): test fix * chore(text update): updates text string * chore(test update): test update --- packages/custodyKeyring/src/CustodyKeyring.ts | 2 +- .../jupiter/JupiterCustodyKeyring.test.ts | 2 +- .../extension/src/ExtensionSelectors.test.ts | 110 ------------------ packages/extension/src/ExtensionSelectors.ts | 48 -------- packages/extension/src/ExtensionUtils.ts | 17 ++- packages/extension/src/index.ts | 16 +-- .../sdk/src/custodianApi/bitgo/BitgoClient.ts | 40 ++++--- .../src/custodianApi/cactus/CactusClient.ts | 34 ++++-- .../custodianApi/json-rpc/JsonRpcClient.ts | 2 +- .../sdk/src/custodianApi/qredo/QredoClient.ts | 28 +++-- packages/types/src/ITransactionDetails.ts | 2 +- 11 files changed, 77 insertions(+), 224 deletions(-) delete mode 100644 packages/extension/src/ExtensionSelectors.test.ts delete mode 100644 packages/extension/src/ExtensionSelectors.ts diff --git a/packages/custodyKeyring/src/CustodyKeyring.ts b/packages/custodyKeyring/src/CustodyKeyring.ts index 67c52fd7..097bde18 100644 --- a/packages/custodyKeyring/src/CustodyKeyring.ts +++ b/packages/custodyKeyring/src/CustodyKeyring.ts @@ -359,7 +359,7 @@ export abstract class CustodyKeyring extends EventEmitter { const normalisedSupportedChainIds = supportedChainIds.map(chainId => Number(chainId).toString()); if (!normalisedSupportedChainIds.includes(chainId)) { - throw new Error(`This network ${chainId} is not supported by ${this.custodianType.name}`); + throw new Error(`This network ${chainId} is not configured or supported with your custody provider.`); } let payload: IEIP1559TxParams | ILegacyTXParams; diff --git a/packages/custodyKeyring/src/custodianTypes/jupiter/JupiterCustodyKeyring.test.ts b/packages/custodyKeyring/src/custodianTypes/jupiter/JupiterCustodyKeyring.test.ts index da057a02..0d562733 100644 --- a/packages/custodyKeyring/src/custodianTypes/jupiter/JupiterCustodyKeyring.test.ts +++ b/packages/custodyKeyring/src/custodianTypes/jupiter/JupiterCustodyKeyring.test.ts @@ -417,7 +417,7 @@ describe("CustodyKeyring", () => { } as unknown as MetamaskTransaction; await expect(custodyKeyring.signTransaction(fromAddress, ethTx, txMeta)).rejects.toThrowError( - "This network 777 is not supported by Jupiter", + "This network 777 is not configured or supported with your custody provider.", ); }); diff --git a/packages/extension/src/ExtensionSelectors.test.ts b/packages/extension/src/ExtensionSelectors.test.ts deleted file mode 100644 index be8e4f9f..00000000 --- a/packages/extension/src/ExtensionSelectors.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { toChecksumAddress } from "@ethereumjs/util"; - -import { - getConfiguredCustodians, - getCustodianIconForAddress, - getCustodyAccountDetails, - getCustodyAccountSupportedChains, - getMmiPortfolioEnabled, - getMmiPortfolioUrl, - getTransactionStatusMap, - getWaitForConfirmDeepLinkDialog, -} from "./ExtensionSelectors"; - -describe("selectors", () => { - const state = { - metamask: { - waitForConfirmDeepLinkDialog: "123", - custodyStatusMaps: "123", - custodyAccountDetails: { - "0x5Ab19e7091dD208F352F8E727B6DCC6F8aBB6275": { - custodianName: "jupiter", - }, - }, - custodianSupportedChains: { - "0x5ab19e7091dd208f352f8e727b6dcc6f8abb6275": { - supportedChains: ["1", "2"], - custodianName: "jupiter", - }, - }, - mmiConfiguration: { - portfolio: { - enabled: true, - url: "https://portfolio.io", - }, - custodians: [ - { - type: "Jupiter", - name: "jupiter", - apiUrl: "https://jupiter-custody.codefi.network", - iconUrl: "images/jupiter.svg", - displayName: "Jupiter Custody", - production: true, - refreshTokenUrl: null, - isNoteToTraderSupported: false, - version: 1, - }, - ], - }, - }, - }; - - describe("getWaitForConfirmDeepLinkDialog", () => { - it("extracts a state property", () => { - const result = getWaitForConfirmDeepLinkDialog(state); - expect(result).toEqual(state.metamask.waitForConfirmDeepLinkDialog); - }); - }); - - describe("getCustodyAccountDetails", () => { - it("extracts a state property", () => { - const result = getCustodyAccountDetails(state); - expect(result).toEqual(state.metamask.custodyAccountDetails); - }); - }); - - describe("getTransactionStatusMap", () => { - it("extracts a state property", () => { - const result = getTransactionStatusMap(state); - expect(result).toEqual(state.metamask.custodyStatusMaps); - }); - }); - - describe("getCustodianSupportedChains", () => { - it("extracts a state property", () => { - const result = getCustodyAccountSupportedChains(state, "0x5ab19e7091dd208f352f8e727b6dcc6f8abb6275"); - expect(result).toEqual( - state.metamask.custodianSupportedChains[toChecksumAddress("0x5ab19e7091dd208f352f8e727b6dcc6f8abb6275")], - ); - }); - }); - - describe("getMmiPortfolioEnabled", () => { - it("extracts a state property", () => { - const result = getMmiPortfolioEnabled(state); - expect(result).toEqual(state.metamask.mmiConfiguration.portfolio.enabled); - }); - }); - - describe("getMmiPortfolioUrl", () => { - it("extracts a state property", () => { - const result = getMmiPortfolioUrl(state); - expect(result).toEqual(state.metamask.mmiConfiguration.portfolio.url); - }); - }); - - describe("getConfiguredCustodians", () => { - it("extracts a state property", () => { - const result = getConfiguredCustodians(state); - expect(result).toEqual(state.metamask.mmiConfiguration.custodians); - }); - }); - - describe("getCustodianIconForAddress", () => { - it("extracts a state property", () => { - const result = getCustodianIconForAddress(state, "0x5ab19e7091dd208f352f8e727b6dcc6f8abb6275"); - - expect(result).toEqual(state.metamask.mmiConfiguration.custodians[0].iconUrl); - }); - }); -}); diff --git a/packages/extension/src/ExtensionSelectors.ts b/packages/extension/src/ExtensionSelectors.ts deleted file mode 100644 index f845adf2..00000000 --- a/packages/extension/src/ExtensionSelectors.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { toChecksumAddress } from "@ethereumjs/util"; -import { CustodyAccountDetails } from "@metamask-institutional/custody-controller"; -import { IJsonRpcCustodian } from "@metamask-institutional/custody-keyring"; -import { ITransactionStatusMap } from "@metamask-institutional/types"; - -export function getWaitForConfirmDeepLinkDialog(state): boolean { - return state.metamask.waitForConfirmDeepLinkDialog; -} - -export function getTransactionStatusMap(state): ITransactionStatusMap { - return state.metamask.custodyStatusMaps; -} - -export function getCustodyAccountDetails(state): { [key: string]: CustodyAccountDetails } { - return state.metamask.custodyAccountDetails; -} - -export function getCustodyAccountSupportedChains(state, address) { - return state.metamask.custodianSupportedChains - ? state.metamask.custodianSupportedChains[toChecksumAddress(address)] - : []; -} - -export function getMmiPortfolioEnabled(state): boolean { - return state.metamask.mmiConfiguration?.portfolio?.enabled; -} - -export function getMmiPortfolioUrl(state): string { - return state.metamask.mmiConfiguration?.portfolio?.url; -} - -export function getConfiguredCustodians(state): IJsonRpcCustodian[] { - return state.metamask.mmiConfiguration?.custodians || []; -} - -export function getCustodianIconForAddress(state, address: string): string { - let custodianIcon; - - const checksummedAddress = toChecksumAddress(address); - if (state.metamask.custodyAccountDetails?.[checksummedAddress]) { - const { custodianName } = state.metamask.custodyAccountDetails[checksummedAddress]; - custodianIcon = state.metamask.mmiConfiguration?.custodians?.find( - custodian => custodian.name === custodianName, - )?.iconUrl; - } - - return custodianIcon; -} diff --git a/packages/extension/src/ExtensionUtils.ts b/packages/extension/src/ExtensionUtils.ts index 740c21ac..9bbb3f72 100644 --- a/packages/extension/src/ExtensionUtils.ts +++ b/packages/extension/src/ExtensionUtils.ts @@ -295,34 +295,32 @@ export async function handleTxStatusUpdate( }; txMeta.txParams = newTxParams; } - + // Sometimes custodians do not send a webhook when a transaction is submitted - // They may only send one when the TX is confirmed (e.g. Safe) so we need to handle // submission the same as confirmation // Metamask resolves the promise in transaction controller when the TX is submitted - // So we can yield control back to that function as early as we can + // So we can yield control back to that function as early as we can - /** - * Previously, we were waiting for transaction confirmation from custodians before yielding + /** + * Previously, we were waiting for transaction confirmation from custodians before yielding * We are now yielding on submission * This has two effects * 1. Interaction is yieled by MMI back to the dapp before confirmation, meaning it is much quicker * 2. On-chain failures do not cause the dapp's RPC promise to be rejected, which is more in line with MetaMask's behaviour - * */ - + * */ if ( txData.transaction.status.submitted || (txData.transaction.status.finished && - txData.transaction.status.success && - txMeta.status !== MetaMaskTransactionStatuses.CONFIRMED) + txData.transaction.status.success && + txMeta.status !== MetaMaskTransactionStatuses.CONFIRMED) ) { // We don't need to wait for the custodian to confirm the TX, since MM's internal // block tracker will do that for us txStateManager.setTxStatusSubmitted(txMeta.id); - } else if (txData.transaction.status.finished && !txData.transaction.status.success) { let message = `Transaction status from custodian: ${txMeta.custodyStatusDisplayText}`; // Clever English language hack IMO @@ -338,4 +336,3 @@ export async function handleTxStatusUpdate( } return null; } - diff --git a/packages/extension/src/index.ts b/packages/extension/src/index.ts index b710e510..64a2e966 100644 --- a/packages/extension/src/index.ts +++ b/packages/extension/src/index.ts @@ -1,17 +1,3 @@ -export { - updateCustodianTransactions, - custodianEventHandlerFactory, - showCustodianDeepLink, -} from "./ExtensionUtils"; - -export { - getWaitForConfirmDeepLinkDialog, - getTransactionStatusMap, - getCustodyAccountSupportedChains, - getMmiPortfolioEnabled, - getMmiPortfolioUrl, - getConfiguredCustodians, - getCustodianIconForAddress, -} from "./ExtensionSelectors"; +export { updateCustodianTransactions, custodianEventHandlerFactory, showCustodianDeepLink } from "./ExtensionUtils"; export { mmiActionsFactory } from "./ExtensionActions"; diff --git a/packages/sdk/src/custodianApi/bitgo/BitgoClient.ts b/packages/sdk/src/custodianApi/bitgo/BitgoClient.ts index f86493a6..30020295 100644 --- a/packages/sdk/src/custodianApi/bitgo/BitgoClient.ts +++ b/packages/sdk/src/custodianApi/bitgo/BitgoClient.ts @@ -33,15 +33,15 @@ export class BitgoClient { const headers = this.getHeaders(); try { - const body = await fetch(`${this.bitgoApiurl}/wallets`, { + const response = await fetch(`${this.bitgoApiurl}/wallets`, { headers, }); - if (!body.ok) { - throw new Error('Error fetching wallet accounts'); + if (!response.ok) { + throw new Error(`Error fetching wallet accounts. Status: ${response.status} ${response.statusText}`); } - const accounts: IBitgoGetEthereumAccountsResponse = await body.json(); + const accounts: IBitgoGetEthereumAccountsResponse = await response.json(); return accounts.data; } catch (e) { throw new CustodianApiError(e); @@ -52,15 +52,17 @@ export class BitgoClient { const headers = this.getHeaders(); try { - const body = await fetch(`${this.bitgoApiurl}/mmi/wallets/address/${address}`, { + const response = await fetch(`${this.bitgoApiurl}/mmi/wallets/address/${address}`, { headers, }); - if (!body.ok) { - throw new Error(`Error fetching account for address ${address}`); + if (!response.ok) { + throw new Error( + `Error fetching account for address ${address}. Status: ${response.status} ${response.statusText}`, + ); } - const accounts: IBitgoGetEthereumAccountsResponse = await body.json(); + const accounts: IBitgoGetEthereumAccountsResponse = await response.json(); if (accounts.data.length) { return accounts.data[0]; } else { @@ -97,7 +99,7 @@ export class BitgoClient { ); if (!response.ok) { - throw new Error('Error creating transaction'); + throw new Error(`Error creating transaction. Status: ${response.status} ${response.statusText}`); } const resultWrapper: IBitgoCreateTransactionResponse = await response.json(); @@ -116,7 +118,9 @@ export class BitgoClient { }); if (!response.ok) { - throw new Error(`Error getting transaction with id ${custodian_transactionId}`); + throw new Error( + `Error getting transaction with id ${custodian_transactionId}. Status: ${response.status} ${response.statusText}`, + ); } const transaction = await response.json(); @@ -135,7 +139,7 @@ export class BitgoClient { }); if (!response.ok) { - throw new Error('Error getting transactions'); + throw new Error(`Error getting transactions. Status: ${response.status} ${response.statusText}`); } const allTransactions = await response.json(); @@ -158,7 +162,7 @@ export class BitgoClient { }); if (!response.ok) { - throw new Error('Error getting Custommer Proof'); + throw new Error(`Error getting Custommer Proof. Status: ${response.status} ${response.statusText}`); } const customerProof = await response.json(); @@ -191,7 +195,9 @@ export class BitgoClient { }); if (!response.ok) { - throw new Error('Error doing signTypedData'); + throw new Error( + `Error doing signTypedData from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -222,7 +228,9 @@ export class BitgoClient { }); if (!response.ok) { - throw new Error('Error doing signPersonalMessage'); + throw new Error( + `Error doing signPersonalMessage from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -248,7 +256,9 @@ export class BitgoClient { ); if (!response.ok) { - throw new Error(`Error getting signed message with id ${custodian_signedMessageId}`); + throw new Error( + `Error getting signed message with id ${custodian_signedMessageId}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); diff --git a/packages/sdk/src/custodianApi/cactus/CactusClient.ts b/packages/sdk/src/custodianApi/cactus/CactusClient.ts index 1b649f6f..803f3f75 100644 --- a/packages/sdk/src/custodianApi/cactus/CactusClient.ts +++ b/packages/sdk/src/custodianApi/cactus/CactusClient.ts @@ -44,7 +44,7 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error fetching the access token'); + throw new Error(`Error fetching the access token. Status: ${response.status} ${response.statusText}`); } const data: ICactusAccessTokenResponse = await response.json(); @@ -68,7 +68,7 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error fetching accounts'); + throw new Error(`Error fetching accounts. Status: ${response.status} ${response.statusText}`); } const accounts: ICactusEthereumAccount[] = await response.json(); @@ -108,7 +108,7 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error creating transaction'); + throw new Error(`Error creating transaction. Status: ${response.status} ${response.statusText}`); } const data = await response.json(); @@ -127,9 +127,11 @@ export class CactusClient { }); if (!response.ok) { - throw new Error(`Error getting signed message with id ${custodian_signedMessageId}`); + throw new Error( + `Error getting signed message with id ${custodian_signedMessageId}. Status: ${response.status} ${response.statusText}`, + ); } - + const data = await response.json(); if (data.length) { @@ -151,7 +153,9 @@ export class CactusClient { }); if (!response.ok) { - throw new Error(`Error getting transaction with id ${custodian_transactionId}`); + throw new Error( + `Error getting transaction with id ${custodian_transactionId}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -174,7 +178,9 @@ export class CactusClient { }); if (!response.ok) { - throw new Error(`Error getting transactions with chainId ${chainId}`); + throw new Error( + `Error getting transactions with chainId ${chainId}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -196,7 +202,7 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error getting Custommer Proof'); + throw new Error(`Error getting Custommer Proof. Status: ${response.status} ${response.statusText}`); } const customerProof = await response.json(); @@ -234,7 +240,9 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error doing signTypedData'); + throw new Error( + `Error doing signTypedData from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -263,7 +271,9 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error doing signPersonalMessage'); + throw new Error( + `Error doing signPersonalMessage from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -282,9 +292,9 @@ export class CactusClient { }); if (!response.ok) { - throw new Error('Error getting chainIds'); + throw new Error(`Error getting chainIds. Status: ${response.status} ${response.statusText}`); } - + const data: ICactusChainIdsResponse = await response.json(); return data; } catch (e) { diff --git a/packages/sdk/src/custodianApi/json-rpc/JsonRpcClient.ts b/packages/sdk/src/custodianApi/json-rpc/JsonRpcClient.ts index d55a7704..ef363313 100644 --- a/packages/sdk/src/custodianApi/json-rpc/JsonRpcClient.ts +++ b/packages/sdk/src/custodianApi/json-rpc/JsonRpcClient.ts @@ -91,7 +91,7 @@ export class JsonRpcClient extends EventEmitter { oldRefreshToken: hashedToken, }); - throw new Error("Custodian session expired"); + throw new Error("Custodian session expired"); // 401 Unauthorized } const responseJson = await response.json(); diff --git a/packages/sdk/src/custodianApi/qredo/QredoClient.ts b/packages/sdk/src/custodianApi/qredo/QredoClient.ts index 94b61110..bc3ff475 100644 --- a/packages/sdk/src/custodianApi/qredo/QredoClient.ts +++ b/packages/sdk/src/custodianApi/qredo/QredoClient.ts @@ -44,9 +44,9 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error fetching the access token'); + throw new Error(`Error fetching the access token. Status: ${response.status} ${response.statusText}`); } - + const data = (await response.json()) as IQredoAccessTokenResponse; if (!data.access_token) { @@ -87,7 +87,7 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error fetching wallet accounts'); + throw new Error(`Error fetching wallet accounts. Status: ${response.status} ${response.statusText}`); } const data = (await response.json()) as IQredoWalletsResponse; @@ -129,7 +129,7 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error creating transaction'); + throw new Error(`Error creating transaction. Status: ${response.status} ${response.statusText}`); } const data = (await response.json()) as IQredoTransaction; @@ -149,7 +149,9 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error(`Error getting transaction with id ${custodian_transactionId}`); + throw new Error( + `Error getting transaction with id ${custodian_transactionId}. Status: ${response.status} ${response.statusText}`, + ); } const data = (await response.json()) as IQredoTransaction; @@ -175,7 +177,9 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error(`Error getting signed message with id ${custodian_signedMessageId}`); + throw new Error( + `Error getting signed message with id ${custodian_signedMessageId}. Status: ${response.status} ${response.statusText}`, + ); } const data = (await response.json()) as IQredoSignatureResponse; @@ -196,7 +200,7 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error getting Custommer Proof'); + throw new Error(`Error getting Custommer Proof. Status: ${response.status} ${response.statusText}`); } const data = (await response.json()) as IQredoCustomerProof; @@ -221,7 +225,7 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error getting Networks'); + throw new Error(`Error getting Networks. Status: ${response.status} ${response.statusText}`); } const data = (await response.json()) as IQredoNetworksResponse; @@ -248,7 +252,9 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error doing signTypedData'); + throw new Error( + `Error doing signTypedData from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); @@ -274,7 +280,9 @@ export class QredoClient extends EventEmitter { }); if (!response.ok) { - throw new Error('Error doing signPersonalMessage'); + throw new Error( + `Error doing signPersonalMessage from address: ${fromAddress}. Status: ${response.status} ${response.statusText}`, + ); } const data = await response.json(); diff --git a/packages/types/src/ITransactionDetails.ts b/packages/types/src/ITransactionDetails.ts index 4b5f3af2..74c99467 100644 --- a/packages/types/src/ITransactionDetails.ts +++ b/packages/types/src/ITransactionDetails.ts @@ -19,7 +19,7 @@ export interface ITransactionDetails { transactionStatusDisplayText?: string; // Optional because it's used for displayText from custodian transaction transactionId?: string; - + chainId?: number; custodianPublishesTransaction?: boolean; signedRawTransaction?: string;