Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Refactor/9083 logger class #9307

Merged
merged 6 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/components/Nav/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@ const App = ({ userLoggedIn }) => {

if (error) {
// Log error for analytics and continue handling deeplink
Logger.error('Error from Branch: ' + error);
const branchError = new Error(error);
Logger.error(branchError, 'Error subscribing to branch.');
}

if (sdkInit.current) {
Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/Ramp/hooks/useFetchRampNetworks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ function useFetchRampNetworks() {
} catch (requestError) {
setError(requestError as Error);
Logger.error(
'useFetchOnRampNetworks::getNetworks',
requestError as Error,
'useFetchOnRampNetworks::getNetworks',
);
} finally {
setIsLoading(false);
Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/Ramp/hooks/useRampNetworksDetail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ function useRampNetworksDetail() {
} catch (requestError) {
setError(requestError as Error);
Logger.error(
'useRampNetworksDetail::getNetworksDetails',
requestError as Error,
'useRampNetworksDetail::getNetworksDetails',
);
} finally {
setIsLoading(false);
Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/Ramp/orderProcessor/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('processOrder', () => {
unsupportedProviderOrder,
);
expect(Logger.error).toHaveBeenCalledWith(
'FiatOrders::ProcessOrder unrecognized provider',
new Error('FiatOrders::ProcessOrder unrecognized provider'),
unsupportedProviderOrder,
);
});
Expand Down
5 changes: 4 additions & 1 deletion app/components/UI/Ramp/orderProcessor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ function processOrder(
return processAggregatorOrder(order, options);
}
default: {
Logger.error('FiatOrders::ProcessOrder unrecognized provider', order);
const unrecognizedProviderError = new Error(
'FiatOrders::ProcessOrder unrecognized provider',
);
Logger.error(unrecognizedProviderError, order);
return order;
}
}
Expand Down
3 changes: 2 additions & 1 deletion app/components/Views/LockScreen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ class LockScreen extends PureComponent {
// Check https://github.com/MetaMask/mobile-planning/issues/1507
const { selectedAddress } = this.props;
if (typeof selectedAddress !== 'string') {
Logger.error('unlockKeychain error', 'selectedAddress is not a string');
const unlockKeychainError = new Error('unlockKeychain error');
Logger.error(unlockKeychainError, 'selectedAddress is not a string');
}
await Authentication.appTriggeredAuth({
selectedAddress: this.props.selectedAddress,
Expand Down
8 changes: 5 additions & 3 deletions app/components/Views/Login/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ class Login extends PureComponent {
// Check https://github.com/MetaMask/mobile-planning/issues/1507
const { selectedAddress } = this.props;
if (typeof selectedAddress !== 'string') {
Logger.error('Login error', 'selectedAddress is not a string');
const loginError = new Error('Login error');
Logger.error(loginError, 'selectedAddress is not a string');
}

await Authentication.userEntryAuth(
Expand Down Expand Up @@ -435,7 +436,7 @@ class Login extends PureComponent {
} else {
this.setState({ loading: false, error });
}
Logger.error(error, 'Failed to unlock');
Logger.error(e, 'Failed to unlock');
}
};

Expand All @@ -448,7 +449,8 @@ class Login extends PureComponent {
// Check https://github.com/MetaMask/mobile-planning/issues/1507
const { selectedAddress } = this.props;
if (typeof selectedAddress !== 'string') {
Logger.error('unlockKeychain error', 'selectedAddress is not a string');
const unlockKeychainError = new Error('unlockKeychain error');
Logger.error(unlockKeychainError, 'selectedAddress is not a string');
}
await Authentication.appTriggeredAuth({
selectedAddress: this.props.selectedAddress,
Expand Down
5 changes: 4 additions & 1 deletion app/components/Views/ManualBackupStep1/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ const ManualBackupStep1 = ({ route, navigation, appTheme }) => {
setView(CONFIRM_PASSWORD);
}
} catch (e) {
Logger.error('Error trying to recover SRP from keyring-controller');
const srpRecoveryError = new Error(
'Error trying to recover SRP from keyring-controller',
);
Logger.error(srpRecoveryError);
setView(CONFIRM_PASSWORD);
}
}
Expand Down
3 changes: 2 additions & 1 deletion app/components/Views/RestoreWallet/WalletRestored.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ const WalletRestored = () => {
// Log to provide insights into bug research.
// Check https://github.com/MetaMask/mobile-planning/issues/1507
if (typeof selectedAddress !== 'string') {
Logger.error('Wallet restore error', 'selectedAddress is not a string');
const walletRestoreError = new Error('Wallet restore error');
Logger.error(walletRestoreError, 'selectedAddress is not a string');
}
await Authentication.appTriggeredAuth({ selectedAddress });
navigation.replace(Routes.ONBOARDING.HOME_NAV);
Expand Down
3 changes: 2 additions & 1 deletion app/components/Views/Root/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export default class Root extends PureComponent {
constructor(props) {
super(props);
if (props.foxCode === '') {
Logger.error('WARN - foxCode is an empty string');
const foxCodeError = new Error('WARN - foxCode is an empty string');
Logger.error(foxCodeError);
}
SecureKeychain.init(props.foxCode);
// Init EntryScriptWeb3 asynchronously on the background
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class NetworkSettings extends PureComponent {
try {
endpointChainId = await jsonRpcRequest(rpcUrl, 'eth_chainId');
} catch (err) {
Logger.error('Failed to fetch the chainId from the endpoint.', err);
Logger.error(err, 'Failed to fetch the chainId from the endpoint.');
providerError = err;
}

Expand All @@ -488,10 +488,10 @@ class NetworkSettings extends PureComponent {
try {
endpointChainId = new BigNumber(endpointChainId, 16).toString(10);
} catch (err) {
Logger.error(
'Failed to convert endpoint chain ID to decimal',
Logger.error(err, {
endpointChainId,
);
message: 'Failed to convert endpoint chain ID to decimal',
});
}
}

Expand Down
9 changes: 6 additions & 3 deletions app/core/BackupVault/backupVault.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ export async function backupVault(
options,
);
if (backupResult === false) {
Logger.error(VAULT_BACKUP_KEY, VAULT_BACKUP_FAILED);
const vaultBackupFailedError = new Error(VAULT_BACKUP_KEY);
Logger.error(vaultBackupFailedError, VAULT_BACKUP_FAILED);
const response: KeyringBackupResponse = {
success: false,
error: VAULT_BACKUP_FAILED,
Expand All @@ -58,7 +59,8 @@ export async function backupVault(
};
return response;
}
Logger.error(VAULT_BACKUP_KEY, VAULT_BACKUP_FAILED_UNDEFINED);
const vaultBackupUndefinedError = new Error(VAULT_BACKUP_KEY);
Logger.error(vaultBackupUndefinedError, VAULT_BACKUP_FAILED_UNDEFINED);
const response: KeyringBackupResponse = {
success: false,
error: VAULT_BACKUP_FAILED_UNDEFINED,
Expand All @@ -80,7 +82,8 @@ export async function getVaultFromBackup(): Promise<KeyringBackupResponse> {
if (credentials) {
return { success: true, vault: credentials.password };
}
Logger.error(VAULT_BACKUP_KEY, VAULT_FAILED_TO_GET_VAULT_FROM_BACKUP);
const vaultFetchError = new Error(VAULT_BACKUP_KEY);
Logger.error(vaultFetchError, VAULT_FAILED_TO_GET_VAULT_FROM_BACKUP);
return { success: false, error: VAULT_FAILED_TO_GET_VAULT_FROM_BACKUP };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function handleUniversalLink({
SDKConnect.getInstance()
.bindAndroidSDK()
.catch((err) => {
Logger.error(`DeepLinkManager failed to connect`, err);
Logger.error(err, `DeepLinkManager failed to connect`);
});
return;
}
Expand All @@ -62,7 +62,7 @@ function handleUniversalLink({
otherPublicKey: params.pubkey,
sdkConnect: SDKConnect.getInstance(),
}).catch((err: unknown) => {
Logger.error(`DeepLinkManager failed to connect`, err);
Logger.error(err as Error, `DeepLinkManager failed to connect`);
});
}
return true;
Expand Down
5 changes: 4 additions & 1 deletion app/core/DeeplinkManager/ParseManager/parseDeeplink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ function parseDeeplink({

return true;
} catch (error) {
Logger.error(error, 'DeepLinkManager:parse error parsing deeplink');
Logger.error(
error as Error,
'DeepLinkManager:parse error parsing deeplink',
);

if (error) {
Alert.alert(strings('deeplink.invalid'), `Invalid URL: ${url}`);
Expand Down
2 changes: 1 addition & 1 deletion app/core/RPCMethods/RPCMethodMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export const getRpcMethodMiddleware = ({
},
);
handle?.catch((error) => {
Logger.error('Failed to get permissions', error);
Logger.error(error as Error, 'Failed to get permissions');
});
}),
wallet_requestPermissions: async () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function handleClientsReady({
updateOriginatorInfos,
approveHost,
onError: (error) => {
Logger.error(error, '');
Logger.error(error as Error, '');

instance.setLoading(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export default class DeeplinkProtocolService {
await Linking.openURL(deeplink);
} catch (error) {
Logger.error(
error,
error as Error,
`DeeplinkProtocolService::openDeeplink error opening deeplink`,
);
}
Expand Down Expand Up @@ -290,10 +290,10 @@ export default class DeeplinkProtocolService {
request?: string;
}) {
if (!params.originatorInfo) {
Logger.error(
const deepLinkError = new Error(
'DeeplinkProtocolService::handleConnection no originatorInfo',
params,
);
Logger.error(deepLinkError, params);

return;
}
Expand Down
2 changes: 1 addition & 1 deletion app/core/SDKConnect/handlers/handleDeeplink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const handleDeeplink = async ({
});
}
} catch (error) {
Logger.error(error, 'Failed to connect to channel');
Logger.error(error as Error, 'Failed to connect to channel');
}
};

Expand Down
7 changes: 5 additions & 2 deletions app/core/WalletConnect/WalletConnectV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ class WalletConnect2Session {
);
}
} catch (error) {
Logger.error(`WC2::constructor error while handling request`, error);
Logger.error(
error as Error,
`WC2::constructor error while handling request`,
);
}
});
}
Expand Down Expand Up @@ -649,7 +652,7 @@ export class WC2Manager {
await wait(1000);
this.instance = new WC2Manager(web3Wallet, deeplinkSessions, navigation);
} catch (error) {
Logger.error(`WC2@init() failed to create instance`, error);
Logger.error(error as Error, `WC2@init() failed to create instance`);
}

return this.instance;
Expand Down
7 changes: 4 additions & 3 deletions app/lib/ppom/PPOMView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ export class PPOMView extends Component {
this.invoke.define('console.log', (...args: any[]) =>
Logger.log('[PPOMView]', ...args),
);
this.invoke.define('console.error', (...args: any[]) =>
Logger.error('[PPOMView]', args),
);
this.invoke.define('console.error', (...args: any[]) => {
const PPOMError = new Error('[PPOMView]');
return Logger.error(PPOMError, args);
});
this.invoke.define('console.warn', (...args: any[]) =>
Logger.log('[PPOMView]', ...args),
);
Expand Down
99 changes: 99 additions & 0 deletions app/util/Logger/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import Logger from '.';
import {
captureException,
withScope,
captureMessage,
} from '@sentry/react-native';
import { AGREED, METRICS_OPT_IN } from '../../constants/storage';
import DefaultPreference from 'react-native-default-preference';

jest.mock('@sentry/react-native', () => ({
captureException: jest.fn(),
captureMessage: jest.fn(),
withScope: jest.fn(),
}));
const mockedCaptureException = jest.mocked(captureException);
const mockedCaptureMessage = jest.mocked(captureMessage);
const mockedWithScope = jest.mocked(withScope);

describe('Logger', () => {
beforeEach(() => {
DefaultPreference.get = jest.fn((key: string) => {
switch (key) {
case METRICS_OPT_IN:
return Promise.resolve(AGREED);
default:
return Promise.resolve('');
}
});
});

afterEach(() => {
jest.resetAllMocks();
});

describe('error', () => {
it('warns if error is not defined', async () => {
const warn = jest.spyOn(console, 'warn');
await Logger.error(undefined as any);
expect(warn).toBeCalledWith('No error provided');
});

it('skips captureException if metrics is opted out', async () => {
DefaultPreference.get = jest.fn((key: string) => {
switch (key) {
case METRICS_OPT_IN:
return Promise.resolve('');
default:
return Promise.resolve('');
}
});
const testError = new Error('testError');
await Logger.error(testError);
expect(mockedCaptureException).not.toBeCalled();
});

it('calls captureException if metrics is opted in', async () => {
const testError = new Error('testError');
await Logger.error(testError);
expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error));
});

it('calls withScope if extra is passed in', async () => {
const testError = new Error('testError');
await Logger.error(testError, 'extraMessage');
expect(mockedWithScope).toHaveBeenCalledTimes(1);
});

it('calls withScope if extra is passed in', async () => {

Check warning on line 68 in app/util/Logger/index.test.ts

View workflow job for this annotation

GitHub Actions / scripts (lint)

Test title is used multiple times in the same describe block
const testError = new Error('testError');
await Logger.error(testError, 'extraMessage');
expect(mockedWithScope).toHaveBeenCalledTimes(1);
});

it('calls captureException when string is passed instead of Error object', async () => {
const testError = 'testError' as any;
await Logger.error(testError);
expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error));
});
});

describe('message', () => {
it('skips captureMessage if metrics is opted out', async () => {
DefaultPreference.get = jest.fn((key: string) => {
switch (key) {
case METRICS_OPT_IN:
return Promise.resolve('');
default:
return Promise.resolve('');
}
});
await Logger.message('testMessage');
expect(mockedCaptureMessage).not.toHaveBeenCalled();
});
it('calls captureMessage if metrics is opted in', async () => {
await Logger.message('testMessage');
expect(mockedCaptureMessage).toHaveBeenCalledTimes(1);
});
});
});
Loading
Loading