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

Sentry: initialize on app start, and add utility for logging to it #1476

Merged
merged 3 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
15 changes: 3 additions & 12 deletions src/GlobalErrorHandler.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import React from 'react'
import PropTypes from 'prop-types'
import * as Sentry from '@sentry/browser'
import PropTypes from 'prop-types'
import GenericError from './components/Error/GenericError'
import DAONotFoundError from './components/Error/DAONotFoundError'
import { network } from './environment'
import { DAONotFound } from './errors'
import { getSentryDsn, getPackageVersion } from './local-settings'
import { isSentryEnabled } from './sentry'
import ErrorScreen from './components/Error/ErrorScreen'

const SENTRY_DSN = getSentryDsn()
const PACKAGE_VERSION = getPackageVersion()

class GlobalErrorHandler extends React.Component {
static propTypes = {
children: PropTypes.node,
Expand All @@ -37,11 +33,6 @@ class GlobalErrorHandler extends React.Component {
window.removeEventListener('hashchange', this.handleHashchange)
}
handleReportClick = () => {
Sentry.init({
dsn: SENTRY_DSN,
release: PACKAGE_VERSION,
environment: network.type,
})
const eventId = Sentry.captureException(this.state.error)
Sentry.showReportDialog({ eventId })
}
Expand All @@ -60,7 +51,7 @@ class GlobalErrorHandler extends React.Component {
<GenericError
detailsTitle={error.message}
detailsContent={errorStack}
reportCallback={SENTRY_DSN ? this.handleReportClick : null}
reportCallback={isSentryEnabled ? this.handleReportClick : null}
/>
)}
</ErrorScreen>
Expand Down
6 changes: 4 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import 'core-js/stable'
import 'regenerator-runtime/runtime'

// Initialize Sentry immediately if enabled
import './sentry'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we feel like doing this is a bit sketchy / hidden? Would it be better if it were done directly in the index.js instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about exporting a initSentry() function that would get called from here? Or is it to capture errors on import for react, react-dom and @aragon/ui?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no good reason :).

I was going back and forth a couple times on it, but agree with you guys that mangling the global space on an import like this is probably less of a good idea.


import React from 'react'
import ReactDOM from 'react-dom'
import { Main } from '@aragon/ui'
Expand All @@ -24,12 +27,11 @@ const lastPackageVersion = getLastPackageVersion()
const [currentMajorVersion, currentMinorVersion] = packageVersion.split('.')
const [lastMajorVersion, lastMinorVersion] = lastPackageVersion.split('.')

// Setting a package version also clears all local storage data.
// Purge localstorage when upgrading between different minor versions.
if (
lastMajorVersion !== currentMajorVersion ||
lastMinorVersion !== currentMinorVersion
) {
// Purge localstorage when upgrading between different minor versions.
window.localStorage.clear()

// Attempt to clean up indexedDB storage as well.
Expand Down
11 changes: 6 additions & 5 deletions src/routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import React, {
useRef,
} from 'react'
import { createHashHistory as createHistory } from 'history'
import { logWithSentry } from './sentry'
import { staticApps } from './static-apps'
import { isAddress, isValidEnsName } from './web3-utils'
import { addStartingSlash, log } from './utils'
import { addStartingSlash } from './utils'

export const ARAGONID_ENS_DOMAIN = 'aragonid.eth'

Expand Down Expand Up @@ -120,7 +121,7 @@ export function getPath({ mode, preferences } = {}) {
let { orgAddress } = mode

if (!orgAddress) {
log(
logWithSentry(
"Routing(path): 'orgAddress' is a required component for 'org' mode. " +
`Defaulted to '${fallbackPath}'.`
)
Expand All @@ -145,7 +146,7 @@ export function getPath({ mode, preferences } = {}) {

let { instancePath = '' } = mode
if (instancePath && !instanceId) {
log(
logWithSentry(
"Routing(path): 'instancePath' can only be provided if an " +
`'instanceId' is provided in 'org' mode. Ignored '${instancePath}'.`
)
Expand All @@ -157,7 +158,7 @@ export function getPath({ mode, preferences } = {}) {
)
}

log(
logWithSentry(
`Routing(path): invalid mode '${mode.name}' set. Defaulted to '${fallbackPath}'.`
)

Expand Down Expand Up @@ -189,7 +190,7 @@ export function parsePreferences(search = '') {
export function getPreferencesSearch({ section, subsection, data = {} } = {}) {
if (!section) {
if (subsection) {
log(
logWithSentry(
"Routing(preferences): 'subsection' can only be provided if 'section' " +
`is provided. Ignored '${subsection}'.`
)
Expand Down
25 changes: 25 additions & 0 deletions src/sentry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as Sentry from '@sentry/browser'
import { network } from './environment'
import { getPackageVersion, getSentryDsn } from './local-settings'
import { log } from './utils'

const packageVersion = getPackageVersion()
const sentryDsn = getSentryDsn()

export const isSentryEnabled = !!sentryDsn

export function logWithSentry(message, level = 'warning') {
log(message)
if (sentryDsn) {
Sentry.captureMessage(message, level)
}
}

// Automatically initialize if enabled
if (sentryDsn) {
Sentry.init({
dsn: sentryDsn,
release: packageVersion,
environment: network.shortName,
})
}
5 changes: 4 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ export function appendTrailingSlash(str) {
}

export function log(...params) {
if (process.env.NODE_ENV !== 'production') {
if (
process.env.NODE_ENV !== 'production' &&
process.env.NODE_ENV !== 'test'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a few tests (routing) that are causing the logger to emit statements during tests, so this turns them off.

) {
console.log(...params)
}
}
Expand Down