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

Update sentry #999

Merged
merged 3 commits into from
Jan 17, 2019
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: 1 addition & 2 deletions .env-template
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ REACT_APP_ENV=production

# REACT_APP_SHARETRIBE_USING_SSL=true
# SERVER_SHARETRIBE_TRUST_PROXY=true
# REACT_APP_PUBLIC_SENTRY_DSN=change-me
# SERVER_SENTRY_DSN=change-me
# REACT_APP_SENTRY_DSN=change-me
# REACT_APP_CSP=report
# BASIC_AUTH_USERNAME=sharetribe
# BASIC_AUTH_PASSWORD=secret
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ way to update this template, but currently, we follow a pattern:

## Upcoming version 2019-XX-XX

- [change] Change from Raven to Sentry SDKs for browser and Node.js to version 4.5.1. With the new
SDKs only one DSN needs to be configured so update also environment variables and documentation
related to Sentry. [#999](https://github.com/sharetribe/flex-template-web/pull/999)
- [fix] Use environment variable `REACT_APP_AVAILABILITY_ENABLED` to enable or disable availability
calendar. In the config.js file variable fetchAvailableTimeSlots is now renamed to more general
enableAvailability because it affects both fetching availability data and enabling the
Expand Down
3 changes: 1 addition & 2 deletions docs/env.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ them have defaults that work for development environment. For production deploys
| REACT_APP_ENV | A more fine grained env definition than NODE_ENV. Is used for example to differentiate envs in logging. |
| REACT_APP_SHARETRIBE_USING_SSL | Redirect HTTP to HTTPS? |
| SERVER_SHARETRIBE_TRUST_PROXY | Set when running the app behind a reverse proxy, e.g. in Heroku. |
| REACT_APP_PUBLIC_SENTRY_DSN | See: [Error logging with Sentry](./sentry.md) |
| SERVER_SENTRY_DSN | See: [Error logging with Sentry](./sentry.md) |
| REACT_APP_SENTRY_DSN | See: [Error logging with Sentry](./sentry.md) |
| REACT_APP_CSP | See: [Content Security Policy (CSP)](./content-security-policy.md) |
| BASIC_AUTH_USERNAME | Set to enable HTTP Basic Auth |
| BASIC_AUTH_PASSWORD | Set to enable HTTP Basic Auth |
Expand Down
13 changes: 6 additions & 7 deletions docs/sentry.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,16 @@ be used but the Sentry client comes already strapped into application.
## Setting up Sentry keys

To enable the Sentry error logging a DSN, _Data Source Name_ has to be provided as an environment
variable. Browser and Node environments both require their own keys. The key names are as follows:
variable. Browser and Node environments use both the same key:

- **SERVER_SENTRY_DSN** - the private Sentry DSN, used on the server side
- **REACT_APP_PUBLIC_SENTRY_DSN** - the public Sentry DSN, used in the browser
- **REACT_APP_SENTRY_DSN**

The DSN keys can be aquired from the Sentry project settings. To test them in your local environment
they can be passed for example to the `yarn run dev-server` command:
The DSN key can be aquired from the Sentry project settings. To test it in your local environment it
can be passed for example to the `yarn run dev-server` command:

REACT_APP_PUBLIC_SENTRY_DSN='<public-sentry-dsn>' SERVER_SENTRY_DSN='<private-sentry-dsn>' yarn run dev-server
REACT_APP_SENTRY_DSN='<sentry-dsn>' yarn run dev-server

If the Sentry DSN keys are not provided the template app will log errors to the console. The logging
If the Sentry DSN key is not provided the template app will log errors to the console. The logging
and Sentry setup is implemented in [util/log.js](../src/util/log.js) and
[server/log.js](../server/log.js) so refer to those files to change the external logging service or
the logging logic in general.
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"license": "Apache-2.0",
"dependencies": {
"@mapbox/polyline": "^1.0.0",
"@sentry/browser": "4.5.1",
"@sentry/node": "4.5.1",
"array-includes": "^3.0.3",
"array.prototype.find": "^2.0.4",
"autosize": "^4.0.0",
Expand Down Expand Up @@ -32,8 +34,6 @@
"prop-types": "^15.6.2",
"query-string": "^5.1.1",
"raf": "3.4.0",
"raven": "^2.6.4",
"raven-js": "^3.27.0",
"react": "^16.6.3",
"react-dates": "^18.2.2",
"react-dom": "^16.6.3",
Expand Down
25 changes: 14 additions & 11 deletions server/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
* or just plain printing errors to the log.
*/

const Raven = require('raven');
const Sentry = require('@sentry/node');

const ENV = process.env.REACT_APP_ENV;
const SENTRY_DSN = process.env.SERVER_SENTRY_DSN;
const SENTRY_DSN = process.env.REACT_APP_SENTRY_DSN;

/**
* Set up error loggin. If a Sentry DSN is defined
Expand All @@ -19,12 +19,7 @@ exports.setup = () => {
// exceptions from starting the server etc. but does not catch the
// ones thrown from Express.js middleware functions. For those
// an error handler has to be added to the Express app.
Raven.config(SENTRY_DSN, {
environment: ENV,
autoBreadcrumbs: {
http: true,
},
}).install();
Sentry.init({ dsn: SENTRY_DSN, environment: ENV });
}
};

Expand All @@ -34,7 +29,7 @@ exports.setup = () => {
*/
exports.requestHandler = () => {
if (SENTRY_DSN) {
return Raven.requestHandler();
return Sentry.Handlers.requestHandler();
} else {
return (req, res, next) => {
next();
Expand All @@ -48,7 +43,7 @@ exports.requestHandler = () => {
*/
exports.errorHandler = () => {
if (SENTRY_DSN) {
return Raven.errorHandler();
return Sentry.Handlers.errorHandler();
} else {
return (err, req, res, next) => {
next(err);
Expand All @@ -67,7 +62,15 @@ exports.errorHandler = () => {
*/
exports.error = (e, code, data) => {
if (SENTRY_DSN) {
Raven.captureException(e, { tags: { code }, extra: data });
const extra = { ...data, apiErrorData: responseApiErrorInfo(e) };

Sentry.withScope(scope => {
scope.setTag('code', code);
Object.keys(extra).forEach(key => {
scope.setExtra(key, extra[key]);
});
Sentry.captureException(e);
});
} else {
console.error(e);
console.error(code);
Expand Down
2 changes: 1 addition & 1 deletion src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const currency = process.env.REACT_APP_SHARETRIBE_MARKETPLACE_CURRENCY;
const listingMinimumPriceSubUnits = 0;

// Sentry DSN (Data Source Name), a client key for authenticating calls to Sentry
const sentryDsn = process.env.REACT_APP_PUBLIC_SENTRY_DSN;
const sentryDsn = process.env.REACT_APP_SENTRY_DSN;

// If webapp is using SSL (i.e. it's behind 'https' protocol)
const usingSSL = process.env.REACT_APP_SHARETRIBE_USING_SSL === 'true';
Expand Down
31 changes: 21 additions & 10 deletions src/util/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
*/

import Raven from 'raven-js';
import * as Sentry from '@sentry/browser';
import config from '../config';
import { responseApiErrorInfo } from './errors';

Expand All @@ -18,7 +18,10 @@ export const setup = () => {
if (config.sentryDsn) {
// Configures the Sentry client. Adds a handler for
// any uncaught exception.
Raven.config(config.sentryDsn, { environment: config.env }).install();
Sentry.init({
dsn: config.sentryDsn,
environment: config.env,
});
}
};

Expand All @@ -29,18 +32,19 @@ export const setup = () => {
* @param {String} userId ID of current user
*/
export const setUserId = userId => {
if (Raven.isSetup()) {
Raven.setUserContext({ id: userId });
}
Sentry.configureScope(scope => {
scope.setUser({ id: userId });
});
};

/**
* Clears the user ID.
*/

export const clearUserId = () => {
if (Raven.isSetup()) {
Raven.setUserContext();
}
Sentry.configureScope(scope => {
scope.remove_user();
});
};

/**
Expand All @@ -53,9 +57,16 @@ export const clearUserId = () => {
* @param {Object} data Additional data to be sent to Sentry
*/
export const error = (e, code, data) => {
if (Raven.isSetup()) {
if (config.sentryDsn) {
const extra = { ...data, apiErrorData: responseApiErrorInfo(e) };
Raven.captureException(e, { tags: { code }, extra });

Sentry.withScope(scope => {
scope.setTag('code', code);
Object.keys(extra).forEach(key => {
scope.setExtra(key, extra[key]);
});
Sentry.captureException(e);
});
Copy link
Contributor

@Gnito Gnito Jan 16, 2019

Choose a reason for hiding this comment

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

I think this should use setExtra instead of just setTag:

const extra = { ...data, apiErrorData: responseApiErrorInfo(e) };

Sentry.withScope(scope => {
  scope.setTag('code', code);
  Object.keys(extra).forEach(key => {
    scope.setExtra(key, extra[key]);
  });
  Sentry.captureException(e);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Change this part also in server/log.js!

} else {
console.error(e); // eslint-disable-line
console.error('Error code:', code, 'data:', data);
Expand Down
Loading