8000 add eslint rule to enforce use of `captureException` only from `@sent… · jonator/sentry-javascript@be1509c · GitHub
[go: up one dir, main page]

Skip to content

Commit be1509c

Browse files
committed
add eslint rule to enforce use of captureException only from @sentry/core
1 parent 65245f4 commit be1509c

File tree

4 files changed

+137
-0
lines changed

4 files changed

+137
-0
lines changed

packages/eslint-config-sdk/src/index.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ module.exports = {
144144
// We want to prevent async await usage in our files to prevent uncessary bundle size. Turned off in tests.
145145
'@sentry-internal/sdk/no-async-await': 'error',
146146

147+
// Enforce internal use of `captureException` from `@sentry/core` rather than `@sentry/browser`, `@sentry/node`,
148+
// or any wrapper SDK thereof. This prevents manual-usage mechanism data from getting erroneously included.
149+
'@sentry-internal/sdk/captureException-from-core': 'error',
150+
147151
// JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation,
148152
// even if it may seems excessive at times, is important to emphasize. Turned off in tests.
149153
'jsdoc/require-jsdoc': [

packages/eslint-plugin-sdk/src/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@ module.exports = {
1212
rules: {
1313
'no-async-await': require('./rules/no-async-await'),
1414
'no-eq-empty': require('./rules/no-eq-empty'),
15+
'captureException-from-core': require('./rules/captureException-from-core'),
1516
},
1617
};
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* Force all internal use of `captureException` to come from `@sentry/core` rather than `@sentry/browser`,
3+
* `@sentry/node`, or any wrapper SDK, in order to prevent accidental inclusion of manual-usage mechansism values.
4+
*
5+
* TODO (maybe): Doesn't catch unpacking of the module object (code like
6+
*
7+
* `import * as Sentry from '@sentry/xxx'; const { captureException } = Sentry; captureException(...);`
8+
*
9+
* ) because it's unlikely we'd do that and the rule would probably be more complicated than it's worth. (There are
10+
* probably other strange ways to call the wrong version of `captureException`, and this rule doesn't catch those,
11+
* either, but again, unlikely to come up in real life.)
12+
*/
13+
14+
/** @type {import('eslint').Rule.RuleModule} */
15+
module.exports = {
16+
meta: {
17+
type: 'problem',
18+
docs: {
19+
description: 'Enforce internal usage of `captureException` from `@sentry/core`',
20+
},
21+
messages: {
22+
errorMessage:
23+
'All internal uses of `captureException` should come from `@sentry/core`, not the browser or node SDKs (or any of their wrappers). (The browser and node versions of `captureException` have manual-capture `mechanism` data baked in, which is probably not what you want.)',
24+
},
25+
},
26+
27+
create: function (context) {
28+
return {
29+
// This catches imports of the form `import { captureException } from '@sentry/xxx';`
30+
ImportDeclaration: function (node) {
31+
if (
32+
node.specifiers.some(
33+
specifier =>
34+
specifier.type === 'ImportSpecifier' &&
35+
specifier.imported.type === 'Identifier' &&
36+
specifier.imported.name === 'captureException',
37+
) &&
38+
node.source.value !== '@sentry/core'
39+
) {
40+
context.report({ node, messageId: 'errorMessage' });
41+
}
42+
},
43+
44+
// This catches uses like `import * as Sentry from '@sentry/xxx'; Sentry.captureException(...);`
45+
CallExpression: function (node) {
46+
if (node.callee.type === 'MemberExpression' && node.callee.property.name === 'captureException') {
47+
// NOTE: In all comments below, "the object" refers to the object (presumably a module) containing `captureException`.
48+
49+
// This is the name of the object. IOW, it's the `Sentry` in `Sentry.captureException`.
50+
const objectName = node.callee.object.name;
51+
52+
// All statements defining the object. (Not entirely clear how there there could be more than one, but
53+
// ¯\_(ツ)_/¯. Note: When we find a reference to the object, it may or may not be the reference in
54+
// `Sentry.captureException`, but we don't care, because we just want to use it to jump back to the original
55+
// definition.)
56+
const objectDefinitions = context
57+
.getScope()
58+
.references.find(reference => reference.identifier && reference.identifier.name === objectName)
59+
.resolved.defs;
60+
61+
// Of the definitions, one which comes as part of an import, if any
62+
const namespaceImportDef = objectDefinitions.find(definition => definition.type === 'ImportBinding');
63+
64+
if (
65+
namespaceImportDef &&
66+
namespaceImportDef.parent.type === 'ImportDeclaration' &&
67+
namespaceImportDef.parent.source.value !== '@sentry/core'
68+
) {
69+
context.report({ node, messageId: 'errorMessage' });
70+
}
71+
}
72+
},
73+
};
74+
},
75+
};
Lines changed: 57 additions & 0 deletions
57AE
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
const { RuleTester } = require('eslint');
2+
3+
const captureExceptionFromCoreRule = require('../../../src/rules/captureException-from-core.js');
4+
const ruleTester = new RuleTester({
5+
parserOptions: {
6+
sourceType: 'module',
7+
ecmaVersion: 2015,
8+
},
9+
});
10+
ruleTester.run('captureException-from-core', captureExceptionFromCoreRule, {
11+
valid: [
12+
{
13+
// Solo import
14+
code: "import { captureException } from '@sentry/core';",
15+
},
16+
{
17+
// One import among many
18+
code: "import { captureException, Hub } from '@sentry/core';",
19+
},
20+
{
21+
// Full module import
22+
code: "import * as SentryCore from '@sentry/core'; SentryCore.captureException('');",
23+
},
24+
{
25+
// Full module import used inside a function
26+
code: "import * as SentryCore from '@sentry/core'; const func = () => SentryCore.captureException('');",
27+
},
28+
],
29+
30+
invalid: [
31+
{
32+
// Solo import from browser SDK
33+
code: "import { captureException } from '@sentry/browser';",
34+
errors: [{ messageId: 'errorMessage' }],
35+
},
36+
{
37+
// Solo import from node SDK
38+
code: "import { captureException } from '@sentry/node';",
39+
errors: [{ messageId: 'errorMessage' }],
40+
},
41+
{
42+
// Solo import from wrapper SDK
43+
code: "import { captureException } from '@sentry/nextjs';",
44+
errors: [{ messageId: 'errorMessage' }],
45+
},
46+
{
47+
// One import among many, from a non-core SDK
48+
code: "import { captureException, showReportDialog } from '@sentry/browser';",
49+
errors: [{ messageId: 'errorMessage' }],
50+
},
51+
{
52+
// Full module import, from a non-core SDK
53+
code: "import * as SentryBrowser from '@sentry/browser'; SentryBrowser.captureException('');",
54+
errors: [{ messageId: 'errorMessage' }],
55+
},
56+
],
57+
});

0 commit comments

Comments
 (0)
0