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

deps(sentry): move from raven to @sentry/node #9325

Merged
merged 15 commits into from
Feb 16, 2022
Merged
2 changes: 1 addition & 1 deletion build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ async function build(entryPath, distPath, opts = {minify: true}) {
'intl-pluralrules',
'intl',
'pako/lib/zlib/inflate.js',
'raven',
'@sentry/node',
'source-map',
'ws',
require.resolve('../lighthouse-core/gather/connections/cri.js'),
Expand Down
5 changes: 1 addition & 4 deletions lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,9 @@ async function begin() {
url: urlUnderTest,
flags: cliFlags,
environmentData: {
name: 'redacted', // prevent sentry from using hostname
serverName: 'redacted', // prevent sentry from using hostname
environment: isDev() ? 'development' : 'production',
release: pkg.version,
tags: {
channel: 'cli',
},
},
});
}
Expand Down
63 changes: 39 additions & 24 deletions lighthouse-core/lib/sentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

const log = require('lighthouse-logger');

/** @typedef {import('raven').CaptureOptions} CaptureOptions */
/** @typedef {import('raven').ConstructorOptions} ConstructorOptions */
/** @typedef {import('@sentry/node').Breadcrumb} Breadcrumb */
/** @typedef {import('@sentry/node').NodeClient} NodeClient */
/** @typedef {import('@sentry/node').NodeOptions} NodeOptions */
/** @typedef {import('@sentry/node').Severity} Severity */

const SENTRY_URL = 'https://a6bb0da87ee048cc9ae2a345fc09ab2e:63a7029f46f74265981b7e005e0f69f8@sentry.io/174697';

Expand All @@ -21,30 +23,30 @@ const SAMPLED_ERRORS = [
// e.g.: {pattern: /No.*node with given id/, rate: 0.01},
];

const noop = () => {};
const noop = () => { };

/**
* A delegate for sentry so that environments without error reporting enabled will use
* noop functions and environments with error reporting will call the actual Sentry methods.
*/
const sentryDelegate = {
init,
/** @type {(message: string, options?: CaptureOptions) => void} */
/** @type {(message: string, level?: Severity) => void} */
captureMessage: noop,
/** @type {(breadcrumb: any) => void} */
/** @type {(breadcrumb: Breadcrumb) => void} */
captureBreadcrumb: noop,
/** @type {() => any} */
getContext: noop,
/** @type {(error: Error, options?: CaptureOptions) => Promise<void>} */
captureException: async () => {},
/** @type {(error: Error, options: {level?: string, tags?: {[key: string]: any}, extra?: {[key: string]: any}}) => Promise<void>} */
captureException: async () => { },
_shouldSample() {
return SAMPLE_RATE >= Math.random();
},
};

/**
* When called, replaces noops with actual Sentry implementation.
* @param {{url: string, flags: LH.CliFlags, environmentData: ConstructorOptions}} opts
* @param {{url: string, flags: LH.CliFlags, environmentData: NodeOptions}} opts
*/
function init(opts) {
// If error reporting is disabled, leave the functions as a noop
Expand All @@ -58,15 +60,25 @@ function init(opts) {
}

try {
const Sentry = require('raven');
const sentryConfig = Object.assign({}, opts.environmentData,
{captureUnhandledRejections: true});
Sentry.config(SENTRY_URL, sentryConfig).install();
const Sentry = require('@sentry/node');
Sentry.init({
...opts.environmentData,
dsn: SENTRY_URL,
});

const extras = {
...opts.flags.throttling,
channel: opts.flags.channel || 'cli',
url: opts.url,
formFactor: opts.flags.formFactor,
throttlingMethod: opts.flags.throttlingMethod,
};
Sentry.setExtras(extras);

// Have each delegate function call the corresponding sentry function by default
sentryDelegate.captureMessage = (...args) => Sentry.captureMessage(...args);
sentryDelegate.captureBreadcrumb = (...args) => Sentry.captureBreadcrumb(...args);
sentryDelegate.getContext = () => Sentry.getContext();
sentryDelegate.captureBreadcrumb = (...args) => Sentry.addBreadcrumb(...args);
sentryDelegate.getContext = () => extras;

// Keep a record of exceptions per audit/gatherer so we can just report once
const sentryExceptionCache = new Map();
Expand Down Expand Up @@ -107,21 +119,24 @@ function init(opts) {
opts.tags.protocolMethod = err.protocolMethod;
}

return new Promise(resolve => {
Sentry.captureException(err, opts, () => resolve());
Sentry.withScope(scope => {
if (opts.level) {
// @ts-expect-error - allow any string.
scope.setLevel(opts.level);
}
if (opts.tags) {
scope.setTags(opts.tags);
}
if (opts.extra) {
scope.setExtras(opts.extra);
}
Sentry.captureException(err);
});
};

const context = Object.assign({
url: opts.url,
formFactor: opts.flags.formFactor,
throttlingMethod: opts.flags.throttlingMethod,
}, opts.flags.throttling);
Sentry.mergeContext({extra: Object.assign({}, opts.environmentData.extra, context)});
} catch (e) {
log.warn(
'sentry',
'Could not load raven library, errors will not be reported.'
'Could not load Sentry, errors will not be reported.'
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class Runner {
Sentry.captureBreadcrumb({
message: 'Run started',
category: 'lifecycle',
data: sentryContext?.extra,
data: sentryContext,
});

/** @type {LH.Artifacts} */
Expand Down
52 changes: 28 additions & 24 deletions lighthouse-core/test/lib/sentry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
*/
'use strict';

jest.mock('raven');
jest.mock('@sentry/node');

const raven = require('raven');
const sentryNode = require('@sentry/node');
const Sentry = require('../../lib/sentry.js');

/* eslint-env jest */
Expand All @@ -27,9 +27,14 @@ describe('Sentry', () => {
// We want to have a fresh state for every test.
originalSentry = {...Sentry};

raven.config = jest.fn().mockReturnValue({install: jest.fn()});
raven.mergeContext = jest.fn();
raven.captureException = jest.fn().mockImplementation((err, opts, cb) => cb());
sentryNode.init = jest.fn().mockReturnValue({install: jest.fn()});
sentryNode.setExtras = jest.fn();
sentryNode.captureException = jest.fn();
sentryNode.withScope = (fn) => fn({
setLevel: () => {},
setTags: () => {},
setExtras: () => {},
});
Sentry._shouldSample = jest.fn().mockReturnValue(true);
});

Expand All @@ -41,18 +46,18 @@ describe('Sentry', () => {
describe('.init', () => {
it('should noop when !enableErrorReporting', () => {
Sentry.init({url: 'http://example.com', flags: {}});
expect(raven.config).not.toHaveBeenCalled();
expect(sentryNode.init).not.toHaveBeenCalled();
Sentry.init({url: 'http://example.com', flags: {enableErrorReporting: false}});
expect(raven.config).not.toHaveBeenCalled();
expect(sentryNode.init).not.toHaveBeenCalled();
});

it('should noop when not picked for sampling', () => {
Sentry._shouldSample.mockReturnValue(false);
Sentry.init({url: 'http://example.com', flags: {enableErrorReporting: true}});
expect(raven.config).not.toHaveBeenCalled();
expect(sentryNode.init).not.toHaveBeenCalled();
});

it('should initialize the raven client when enableErrorReporting', () => {
it('should initialize the Sentry client when enableErrorReporting', () => {
Sentry.init({
url: 'http://example.com',
flags: {
Expand All @@ -63,26 +68,25 @@ describe('Sentry', () => {
environmentData: {},
});

expect(raven.config).toHaveBeenCalled();
expect(raven.mergeContext).toHaveBeenCalled();
expect(raven.mergeContext.mock.calls[0][0]).toEqual({
extra: {
url: 'http://example.com',
formFactor: 'desktop',
throttlingMethod: 'devtools',
},
expect(sentryNode.init).toHaveBeenCalled();
expect(sentryNode.setExtras).toHaveBeenCalled();
expect(sentryNode.setExtras.mock.calls[0][0]).toEqual({
channel: 'cli',
url: 'http://example.com',
formFactor: 'desktop',
throttlingMethod: 'devtools',
});
});
});

describe('.captureException', () => {
it('should forward exceptions to raven client', async () => {
it('should forward exceptions to Sentry client', async () => {
Sentry.init(configPayload);
const error = new Error('oops');
await Sentry.captureException(error);

expect(raven.captureException).toHaveBeenCalled();
expect(raven.captureException.mock.calls[0][0]).toBe(error);
expect(sentryNode.captureException).toHaveBeenCalled();
expect(sentryNode.captureException.mock.calls[0][0]).toBe(error);
});

it('should skip expected errors', async () => {
Expand All @@ -91,7 +95,7 @@ describe('Sentry', () => {
error.expected = true;
await Sentry.captureException(error);

expect(raven.captureException).not.toHaveBeenCalled();
expect(sentryNode.captureException).not.toHaveBeenCalled();
});

it('should skip duplicate audit errors', async () => {
Expand All @@ -100,7 +104,7 @@ describe('Sentry', () => {
await Sentry.captureException(error, {tags: {audit: 'my-audit'}});
await Sentry.captureException(error, {tags: {audit: 'my-audit'}});

expect(raven.captureException).toHaveBeenCalledTimes(1);
expect(sentryNode.captureException).toHaveBeenCalledTimes(1);
});

it('should still allow different audit errors', async () => {
Expand All @@ -110,7 +114,7 @@ describe('Sentry', () => {
await Sentry.captureException(errorA, {tags: {audit: 'my-audit'}});
await Sentry.captureException(errorB, {tags: {audit: 'my-audit'}});

expect(raven.captureException).toHaveBeenCalledTimes(2);
expect(sentryNode.captureException).toHaveBeenCalledTimes(2);
});

it('should skip duplicate gatherer errors', async () => {
Expand All @@ -119,7 +123,7 @@ describe('Sentry', () => {
await Sentry.captureException(error, {tags: {gatherer: 'my-gatherer'}});
await Sentry.captureException(error, {tags: {gatherer: 'my-gatherer'}});

expect(raven.captureException).toHaveBeenCalledTimes(1);
expect(sentryNode.captureException).toHaveBeenCalledTimes(1);
});
});
});
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@
"@types/lodash.set": "^4.3.6",
"@types/node": "*",
"@types/pako": "^1.0.1",
"@types/raven": "^2.5.1",
"@types/resize-observer-browser": "^0.1.1",
"@types/semver": "^5.5.0",
"@types/tabulator-tables": "^4.9.1",
Expand Down Expand Up @@ -183,6 +182,7 @@
"webtreemap-cdt": "^3.2.1"
},
"dependencies": {
"@sentry/node": "^6.17.4",
"axe-core": "4.3.5",
"chrome-launcher": "^0.15.0",
"configstore": "^5.0.1",
Expand All @@ -204,7 +204,6 @@
"open": "^8.4.0",
"parse-cache-control": "1.0.1",
"ps-list": "^8.0.0",
"raven": "^2.2.1",
"robots-parser": "^3.0.0",
"semver": "^5.3.0",
"speedline-core": "^1.4.3",
Expand Down
Loading