From 560ef80c396baea7996c6de2b34270e0ea2ce58c Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Wed, 14 Apr 2021 14:52:11 +0100 Subject: [PATCH 1/7] Move some errors to analytics instead of sentry --- app/components/UI/ComponentErrorBoundary/index.js | 12 +++++++++++- .../UI/Notification/TransactionNotification/index.js | 6 ------ .../TransactionElement/TransactionDetails/index.js | 3 ++- app/components/UI/TransactionElement/index.js | 5 ----- app/components/Views/LockScreen/index.js | 6 +++++- app/components/Views/Login/index.js | 7 +++++++ .../Views/Settings/SecuritySettings/index.js | 7 ++++++- 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/app/components/UI/ComponentErrorBoundary/index.js b/app/components/UI/ComponentErrorBoundary/index.js index a18a1b2da03..c525f000253 100644 --- a/app/components/UI/ComponentErrorBoundary/index.js +++ b/app/components/UI/ComponentErrorBoundary/index.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Logger from '../../../util/Logger'; +import Analytics from '../../../core/Analytics'; class ComponentErrorBoundary extends React.Component { state = { error: null }; @@ -17,7 +18,11 @@ class ComponentErrorBoundary extends React.Component { /** * Function to be called when there is an error */ - onError: PropTypes.func + onError: PropTypes.func, + /** + * Will not track as an error, but still log to analytics + */ + dontTrackAsError: PropTypes.bool }; static getDerivedStateFromError(error) { @@ -28,6 +33,11 @@ class ComponentErrorBoundary extends React.Component { // eslint-disable-next-line no-unused-expressions this.props.onError?.(); + const { componentLabel, dontTrackAsError } = this.props; + + if (dontTrackAsError) { + return Analytics.trackEventWithParameters(componentLabel + ' Error', { error: true }); + } Logger.error(error, { View: this.props.componentLabel, ...errorInfo }); } diff --git a/app/components/UI/Notification/TransactionNotification/index.js b/app/components/UI/Notification/TransactionNotification/index.js index 0cff2f48f06..05c47707ac4 100644 --- a/app/components/UI/Notification/TransactionNotification/index.js +++ b/app/components/UI/Notification/TransactionNotification/index.js @@ -94,7 +94,6 @@ function TransactionNotification(props) { accounts, currentNotification, isInBrowserView, - navigation, notificationAnimated, onClose, transactions, @@ -252,7 +251,6 @@ function TransactionNotification(props) { ({ network: state.engine.backgroundState.NetworkController, frequentRpcList: state.engine.backgroundState.PreferencesController.frequentRpcList }); -export default connect(mapStateToProps)(TransactionDetails); +export default connect(mapStateToProps)(withNavigation(TransactionDetails)); diff --git a/app/components/UI/TransactionElement/index.js b/app/components/UI/TransactionElement/index.js index 79e61ac7e16..dc18f8f66fe 100644 --- a/app/components/UI/TransactionElement/index.js +++ b/app/components/UI/TransactionElement/index.js @@ -59,10 +59,6 @@ const transactionIconReceivedFailed = require('../../../images/transaction-icons class TransactionElement extends PureComponent { static propTypes = { assetSymbol: PropTypes.string, - /** - /* navigation object required to push new views - */ - navigation: PropTypes.object, /** * Asset object (in this case ERC721 token) */ @@ -271,7 +267,6 @@ class TransactionElement extends PureComponent { diff --git a/app/components/Views/LockScreen/index.js b/app/components/Views/LockScreen/index.js index 0e7ada846e3..50f07ed7f43 100644 --- a/app/components/Views/LockScreen/index.js +++ b/app/components/Views/LockScreen/index.js @@ -8,6 +8,7 @@ import SecureKeychain from '../../../core/SecureKeychain'; import { baseStyles } from '../../../styles/common'; import Logger from '../../../util/Logger'; import { NavigationActions } from 'react-navigation'; +import Analytics from '../../../core/Analytics'; const LOGO_SIZE = 175; const styles = StyleSheet.create({ @@ -129,7 +130,10 @@ class LockScreen extends PureComponent { if (this.unlockAttempts <= 3) { this.unlockKeychain(); } else { - Logger.error(error, { message: 'Lockscreen:maxAttemptsReached', attemptNumber: this.unlockAttempts }); + Analytics.trackEventWithParameters('Lockscreen:maxAttemptsReached', { + error: true, + attemptNumber: this.unlockAttempts + }); this.props.navigation.navigate('Login'); } } diff --git a/app/components/Views/Login/index.js b/app/components/Views/Login/index.js index 9ea55a275af..7b8cfff2090 100644 --- a/app/components/Views/Login/index.js +++ b/app/components/Views/Login/index.js @@ -44,6 +44,7 @@ import { passwordRequirementsMet } from '../../../util/password'; import ErrorBoundary from '../ErrorBoundary'; import WarningExistingUserModal from '../../UI/WarningExistingUserModal'; import Icon from 'react-native-vector-icons/FontAwesome'; +import Analytics from '../../../core/Analytics'; const isTextDelete = text => String(text).toLowerCase() === 'delete'; const deviceHeight = Device.getDeviceHeight(); @@ -311,6 +312,12 @@ class Login extends PureComponent { error.toLowerCase() === WRONG_PASSWORD_ERROR_ANDROID.toLowerCase() ) { this.setState({ loading: false, error: strings('login.invalid_password') }); + + Analytics.trackEventWithParameters('Invalid password', { + error: true + }); + + return; } else if (error === PASSCODE_NOT_SET_ERROR) { Alert.alert( 'Security Alert', diff --git a/app/components/Views/Settings/SecuritySettings/index.js b/app/components/Views/Settings/SecuritySettings/index.js index 8416178c9b3..620de2e5d77 100644 --- a/app/components/Views/Settings/SecuritySettings/index.js +++ b/app/components/Views/Settings/SecuritySettings/index.js @@ -393,8 +393,13 @@ class Settings extends PureComponent { } catch (e) { if (e.message === 'Invalid password') { Alert.alert(strings('app_settings.invalid_password'), strings('app_settings.invalid_password_message')); + Analytics.trackEventWithParameters('Invalid password', { + error: true, + view: 'SecuritySettings' + }); + } else { + Logger.error(e, 'SecuritySettings:biometrics'); } - Logger.error(e, 'SecuritySettings:biometrics'); this.setState({ [type]: !enabled, loading: false }); } }; From 4d0b99e94f81ceab8949af2d25fdfd58857b6696 Mon Sep 17 00:00:00 2001 From: Pedro Pablo Aste Kompen Date: Wed, 14 Apr 2021 12:58:41 -0400 Subject: [PATCH 2/7] Add swaps errors --- app/components/UI/AssetOverview/index.js | 2 +- app/components/UI/Swaps/QuotesView.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/components/UI/AssetOverview/index.js b/app/components/UI/AssetOverview/index.js index 87fd44b11df..28531ca0a34 100644 --- a/app/components/UI/AssetOverview/index.js +++ b/app/components/UI/AssetOverview/index.js @@ -208,7 +208,7 @@ class AssetOverview extends PureComponent { try { await SwapsController.fetchTokenWithCache(); } catch (error) { - Logger.error(error, 'Swaps: error while fetching tokens with catche in AssetOverview'); + Logger.error(error, 'Swaps: error while fetching tokens with cache in AssetOverview'); } }; diff --git a/app/components/UI/Swaps/QuotesView.js b/app/components/UI/Swaps/QuotesView.js index 124c82db1b8..e79d0c81e7c 100644 --- a/app/components/UI/Swaps/QuotesView.js +++ b/app/components/UI/Swaps/QuotesView.js @@ -49,7 +49,6 @@ import useModalHandler from '../../Base/hooks/useModalHandler'; import useBalance from './utils/useBalance'; import useGasPrice from './utils/useGasPrice'; import { decodeApproveData } from '../../../util/transactions'; -import Logger from '../../../util/Logger'; const POLLING_INTERVAL = AppConstants.SWAPS.POLLING_INTERVAL; const EDIT_MODE_GAS = 'EDIT_MODE_GAS'; @@ -769,7 +768,6 @@ function SwapsQuotesView({ slippage, custom_slippage: slippage !== AppConstants.SWAPS.DEFAULT_SLIPPAGE }; - Logger.error(error?.description, `Swaps: ${error?.key}`); if (error?.key === swapsUtils.SwapsError.QUOTES_EXPIRED_ERROR) { InteractionManager.runAfterInteractions(() => { const parameters = { @@ -785,6 +783,11 @@ function SwapsQuotesView({ Analytics.trackEventWithParameters(ANALYTICS_EVENT_OPTS.NO_QUOTES_AVAILABLE, {}); Analytics.trackEventWithParameters(ANALYTICS_EVENT_OPTS.NO_QUOTES_AVAILABLE, parameters, true); }); + } else { + Analytics.trackEventWithParameters(`Swaps: ${error?.key}`, { + error: true, + description: error?.description + }); } }, [sourceToken, sourceAmount, destinationToken, hasEnoughTokenBalance, slippage] From 62a1064689b2cfcf28d8716e80ccc1e5d8551fbc Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 16 Apr 2021 12:55:50 +0100 Subject: [PATCH 3/7] Change to log just as 1 error --- .../UI/ComponentErrorBoundary/index.js | 4 +-- app/components/UI/Swaps/QuotesView.js | 6 ++--- app/components/Views/LockScreen/index.js | 11 ++++---- app/components/Views/Login/index.js | 6 ++--- .../Views/Settings/SecuritySettings/index.js | 6 ++--- app/util/analyticsV2.js | 25 ++++++++++++++++++- 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/app/components/UI/ComponentErrorBoundary/index.js b/app/components/UI/ComponentErrorBoundary/index.js index c525f000253..7193cb7346a 100644 --- a/app/components/UI/ComponentErrorBoundary/index.js +++ b/app/components/UI/ComponentErrorBoundary/index.js @@ -1,7 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Logger from '../../../util/Logger'; -import Analytics from '../../../core/Analytics'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; class ComponentErrorBoundary extends React.Component { state = { error: null }; @@ -36,7 +36,7 @@ class ComponentErrorBoundary extends React.Component { const { componentLabel, dontTrackAsError } = this.props; if (dontTrackAsError) { - return Analytics.trackEventWithParameters(componentLabel + ' Error', { error: true }); + return trackErrorAsAnalytics(`Component Error Boundary: ${componentLabel}`, error?.message); } Logger.error(error, { View: this.props.componentLabel, ...errorInfo }); } diff --git a/app/components/UI/Swaps/QuotesView.js b/app/components/UI/Swaps/QuotesView.js index e79d0c81e7c..2d8e0c03564 100644 --- a/app/components/UI/Swaps/QuotesView.js +++ b/app/components/UI/Swaps/QuotesView.js @@ -49,6 +49,7 @@ import useModalHandler from '../../Base/hooks/useModalHandler'; import useBalance from './utils/useBalance'; import useGasPrice from './utils/useGasPrice'; import { decodeApproveData } from '../../../util/transactions'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; const POLLING_INTERVAL = AppConstants.SWAPS.POLLING_INTERVAL; const EDIT_MODE_GAS = 'EDIT_MODE_GAS'; @@ -784,10 +785,7 @@ function SwapsQuotesView({ Analytics.trackEventWithParameters(ANALYTICS_EVENT_OPTS.NO_QUOTES_AVAILABLE, parameters, true); }); } else { - Analytics.trackEventWithParameters(`Swaps: ${error?.key}`, { - error: true, - description: error?.description - }); + trackErrorAsAnalytics(`Swaps: ${error?.key}`, error?.description); } }, [sourceToken, sourceAmount, destinationToken, hasEnoughTokenBalance, slippage] diff --git a/app/components/Views/LockScreen/index.js b/app/components/Views/LockScreen/index.js index 50f07ed7f43..1b8ffdbf32b 100644 --- a/app/components/Views/LockScreen/index.js +++ b/app/components/Views/LockScreen/index.js @@ -8,7 +8,7 @@ import SecureKeychain from '../../../core/SecureKeychain'; import { baseStyles } from '../../../styles/common'; import Logger from '../../../util/Logger'; import { NavigationActions } from 'react-navigation'; -import Analytics from '../../../core/Analytics'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; const LOGO_SIZE = 175; const styles = StyleSheet.create({ @@ -130,10 +130,11 @@ class LockScreen extends PureComponent { if (this.unlockAttempts <= 3) { this.unlockKeychain(); } else { - Analytics.trackEventWithParameters('Lockscreen:maxAttemptsReached', { - error: true, - attemptNumber: this.unlockAttempts - }); + trackErrorAsAnalytics( + 'Lockscreen: Max Attempts Reached', + error?.message, + `Unlock attempts: ${this.unlockAttempts}` + ); this.props.navigation.navigate('Login'); } } diff --git a/app/components/Views/Login/index.js b/app/components/Views/Login/index.js index 7b8cfff2090..8a4ce2ce5f2 100644 --- a/app/components/Views/Login/index.js +++ b/app/components/Views/Login/index.js @@ -44,7 +44,7 @@ import { passwordRequirementsMet } from '../../../util/password'; import ErrorBoundary from '../ErrorBoundary'; import WarningExistingUserModal from '../../UI/WarningExistingUserModal'; import Icon from 'react-native-vector-icons/FontAwesome'; -import Analytics from '../../../core/Analytics'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; const isTextDelete = text => String(text).toLowerCase() === 'delete'; const deviceHeight = Device.getDeviceHeight(); @@ -313,9 +313,7 @@ class Login extends PureComponent { ) { this.setState({ loading: false, error: strings('login.invalid_password') }); - Analytics.trackEventWithParameters('Invalid password', { - error: true - }); + trackErrorAsAnalytics('Login: Invalid Password', error); return; } else if (error === PASSCODE_NOT_SET_ERROR) { diff --git a/app/components/Views/Settings/SecuritySettings/index.js b/app/components/Views/Settings/SecuritySettings/index.js index 620de2e5d77..421a06bb3ad 100644 --- a/app/components/Views/Settings/SecuritySettings/index.js +++ b/app/components/Views/Settings/SecuritySettings/index.js @@ -43,6 +43,7 @@ import { import CookieManager from '@react-native-community/cookies'; import Icon from 'react-native-vector-icons/FontAwesome'; import HintModal from '../../../UI/HintModal'; +import { trackErrorAsAnalytics } from '../../../../util/analyticsV2'; const isIos = Device.isIos(); @@ -393,10 +394,7 @@ class Settings extends PureComponent { } catch (e) { if (e.message === 'Invalid password') { Alert.alert(strings('app_settings.invalid_password'), strings('app_settings.invalid_password_message')); - Analytics.trackEventWithParameters('Invalid password', { - error: true, - view: 'SecuritySettings' - }); + trackErrorAsAnalytics('SecuritySettings: Invalid password', e?.message); } else { Logger.error(e, 'SecuritySettings:biometrics'); } diff --git a/app/util/analyticsV2.js b/app/util/analyticsV2.js index 298b9862b0e..bd5655498ee 100644 --- a/app/util/analyticsV2.js +++ b/app/util/analyticsV2.js @@ -88,7 +88,30 @@ export const trackEventV2 = (eventName, params) => { } }; +/** + * This functions logs errors to analytics instead of sentry. + * The objective is to log errors (that are not errors from our side) like “Invalid Password”. + * An error like this generally means a user inserted the wrong password, so logging to sentry doesn't make sense. + * But we still want to log this to analytics so that we are aware of a rapid increase which may mean it's an error from our side, for example, an error with the encryption library. + * @param {String} view + * @param {String} errorMessage + * @param {String} otherInfo + */ +export const trackErrorAsAnalytics = (view, errorMessage, otherInfo) => { + try { + Analytics.trackEventWithParameters(generateOpt('Error occured'), { + error: true, + view, + errorMessage, + otherInfo + }); + } catch (error) { + Logger.error(error, 'Error logging analytics - trackErrorAsAnalytics'); + } +}; + export default { ANALYTICS_EVENTS: ANALYTICS_EVENTS_V2, - trackEvent: trackEventV2 + trackEvent: trackEventV2, + trackErrorAsAnalytics }; From a6a21d8fd1187f3da9451665deb85da40df3d304 Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 16 Apr 2021 12:57:38 +0100 Subject: [PATCH 4/7] Fix typos --- app/util/analyticsV2.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/util/analyticsV2.js b/app/util/analyticsV2.js index bd5655498ee..453f6fac398 100644 --- a/app/util/analyticsV2.js +++ b/app/util/analyticsV2.js @@ -99,7 +99,7 @@ export const trackEventV2 = (eventName, params) => { */ export const trackErrorAsAnalytics = (view, errorMessage, otherInfo) => { try { - Analytics.trackEventWithParameters(generateOpt('Error occured'), { + Analytics.trackEventWithParameters(generateOpt('Error occurred'), { error: true, view, errorMessage, From 25e0f6127b195abca0771b7770838a39a41d7e1e Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 16 Apr 2021 13:19:01 +0100 Subject: [PATCH 5/7] Log can't reach branch servers as analytics --- app/components/Nav/App/index.js | 7 ++++++- app/util/analyticsV2.js | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/components/Nav/App/index.js b/app/components/Nav/App/index.js index 12309f49053..bfecc8ddaf9 100644 --- a/app/components/Nav/App/index.js +++ b/app/components/Nav/App/index.js @@ -28,6 +28,7 @@ import Engine from '../../../core/Engine'; import Logger from '../../../util/Logger'; import Branch from 'react-native-branch'; import AppConstants from '../../../core/AppConstants'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; /** * Stack navigator responsible for the onboarding process @@ -186,7 +187,11 @@ class App extends PureComponent { handleDeeplinks = async ({ error, params, uri }) => { if (error) { - Logger.error(error, 'Deeplink: Error from Branch'); + if (error === 'Trouble reaching the Branch servers, please try again shortly.') { + trackErrorAsAnalytics('Branch: Trouble reaching servers', error); + } else { + Logger.error(error, 'Deeplink: Error from Branch'); + } } const deeplink = params['+non_branch_link'] || uri || null; try { diff --git a/app/util/analyticsV2.js b/app/util/analyticsV2.js index 453f6fac398..8a678fede36 100644 --- a/app/util/analyticsV2.js +++ b/app/util/analyticsV2.js @@ -93,15 +93,15 @@ export const trackEventV2 = (eventName, params) => { * The objective is to log errors (that are not errors from our side) like “Invalid Password”. * An error like this generally means a user inserted the wrong password, so logging to sentry doesn't make sense. * But we still want to log this to analytics so that we are aware of a rapid increase which may mean it's an error from our side, for example, an error with the encryption library. - * @param {String} view + * @param {String} type * @param {String} errorMessage * @param {String} otherInfo */ -export const trackErrorAsAnalytics = (view, errorMessage, otherInfo) => { +export const trackErrorAsAnalytics = (type, errorMessage, otherInfo) => { try { Analytics.trackEventWithParameters(generateOpt('Error occurred'), { error: true, - view, + type, errorMessage, otherInfo }); From 8acbfa2f67b96c88d7c608e1a79b451510bbbce7 Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 16 Apr 2021 13:31:29 +0100 Subject: [PATCH 6/7] Browser: Failed to resolve ENS name for chainId - log as analytics --- app/components/Views/BrowserTab/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/components/Views/BrowserTab/index.js b/app/components/Views/BrowserTab/index.js index 219a7b8a678..a924b5ec86b 100644 --- a/app/components/Views/BrowserTab/index.js +++ b/app/components/Views/BrowserTab/index.js @@ -66,6 +66,7 @@ import { RPC } from '../../../constants/network'; import RPCMethods from '../../../core/RPCMethods'; import AddCustomNetwork from '../../UI/AddCustomNetwork'; import SwitchCustomNetwork from '../../UI/SwitchCustomNetwork'; +import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; const { HOMEPAGE_URL, USER_AGENT, NOTIFICATION_NAMES } = AppConstants; const HOMEPAGE_HOST = 'home.metamask.io'; @@ -913,8 +914,13 @@ export const BrowserTab = props => { ensIgnoreList.push(hostname); return { url: fullUrl, reload: true }; } - Logger.error(err, 'Failed to resolve ENS name'); - Alert.alert(strings('browser.error'), strings('browser.failed_to_resolve_ens_name')); + if (err?.message?.startsWith('EnsIpfsResolver - no known ens-ipfs registry for chainId')) { + trackErrorAsAnalytics('Browser: Failed to resolve ENS name for chainId', err?.message); + } else { + Logger.error(err, 'Failed to resolve ENS name'); + } + + Alert.alert(strings('browser.failed_to_resolve_ens_name'), err.message); goBack(); } }, From b08e423b60d9b40af22eef459ef6c1feb312b998 Mon Sep 17 00:00:00 2001 From: andrepimenta Date: Fri, 30 Apr 2021 18:25:16 +0100 Subject: [PATCH 7/7] Update tests --- .../__snapshots__/index.test.js.snap | 152 ++++-------------- .../TransactionDetails/index.test.js | 2 +- 2 files changed, 33 insertions(+), 121 deletions(-) diff --git a/app/components/UI/TransactionElement/TransactionDetails/__snapshots__/index.test.js.snap b/app/components/UI/TransactionElement/TransactionDetails/__snapshots__/index.test.js.snap index bd9bdbcb3fe..fae6798ed89 100644 --- a/app/components/UI/TransactionElement/TransactionDetails/__snapshots__/index.test.js.snap +++ b/app/components/UI/TransactionElement/TransactionDetails/__snapshots__/index.test.js.snap @@ -1,125 +1,37 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`TransactionDetails should render correctly 1`] = ` - - - - - Status - - - - - - Date - - - [missing "en.date.months.NaN" translation] NaN at NaN:NaNam - - - - - - - From - - - - - - - - To - - - - - - - - - - + } + transactionDetails={ + Object { + "renderFrom": "0x0", + "renderGas": "21000", + "renderGasPrice": "2", + "renderTo": "0x1", + "renderTotalValue": "2 TKN / 0.001 ETH", + "renderTotalValueFiat": "", + "renderValue": "2 TKN", + "transactionHash": "0x2", + } + } + transactionObject={ + Object { + "networkID": "1", + "status": "confirmed", + "transaction": Object { + "nonce": "", + }, + } + } +/> `; diff --git a/app/components/UI/TransactionElement/TransactionDetails/index.test.js b/app/components/UI/TransactionElement/TransactionDetails/index.test.js index b644919a622..a198e801c93 100644 --- a/app/components/UI/TransactionElement/TransactionDetails/index.test.js +++ b/app/components/UI/TransactionElement/TransactionDetails/index.test.js @@ -52,6 +52,6 @@ describe('TransactionDetails', () => { context: { store: mockStore(initialState) } } ); - expect(wrapper.dive()).toMatchSnapshot(); + expect(wrapper).toMatchSnapshot(); }); });