From 75773a857820cd013f8b492fc3dec5fe1b849324 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 03:56:02 -0800 Subject: [PATCH 01/13] add eslint rule to enforce use of `captureException` only from `@sentry/core` --- packages/eslint-config-sdk/src/index.js | 4 + packages/eslint-plugin-sdk/src/index.js | 1 + .../src/rules/captureException-from-core.js | 199 ++++++++++++++++++ .../lib/rules/captureException-from-core.js | 97 +++++++++ 4 files changed, 301 insertions(+) create mode 100644 packages/eslint-plugin-sdk/src/rules/captureException-from-core.js create mode 100644 packages/eslint-plugin-sdk/test/lib/rules/captureException-from-core.js diff --git a/packages/eslint-config-sdk/src/index.js b/packages/eslint-config-sdk/src/index.js index a405a12fc4bb..10c527549485 100644 --- a/packages/eslint-config-sdk/src/index.js +++ b/packages/eslint-config-sdk/src/index.js @@ -144,6 +144,10 @@ module.exports = { // We want to prevent async await usage in our files to prevent uncessary bundle size. Turned off in tests. '@sentry-internal/sdk/no-async-await': 'error', + // Enforce internal use of `captureException` from `@sentry/core` rather than `@sentry/browser`, `@sentry/node`, + // or any wrapper SDK thereof. This prevents manual-usage mechanism data from getting erroneously included. + '@sentry-internal/sdk/captureException-from-core': 'error', + // JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation, // even if it may seems excessive at times, is important to emphasize. Turned off in tests. 'jsdoc/require-jsdoc': [ diff --git a/packages/eslint-plugin-sdk/src/index.js b/packages/eslint-plugin-sdk/src/index.js index 2f57bcb3a6e2..6523eaacf7b7 100644 --- a/packages/eslint-plugin-sdk/src/index.js +++ b/packages/eslint-plugin-sdk/src/index.js @@ -12,5 +12,6 @@ module.exports = { rules: { 'no-async-await': require('./rules/no-async-await'), 'no-eq-empty': require('./rules/no-eq-empty'), + 'captureException-from-core': require('./rules/captureException-from-core'), }, }; diff --git a/packages/eslint-plugin-sdk/src/rules/captureException-from-core.js b/packages/eslint-plugin-sdk/src/rules/captureException-from-core.js new file mode 100644 index 000000000000..1c8ea5d33446 --- /dev/null +++ b/packages/eslint-plugin-sdk/src/rules/captureException-from-core.js @@ -0,0 +1,199 @@ +/** + * Force all internal use of `captureException` to come from `@sentry/core` rather than `@sentry/browser`, + * `@sentry/node`, or any wrapper SDK, in order to prevent accidental inclusion of manual-usage mechansism values. + * + * TODO (maybe): Doesn't catch unpacking of the module object (code like + * + * `import * as Sentry from '@sentry/xxx'; const { captureException } = Sentry; captureException(...);` + * + * ) because it's unlikely we'd do that and the rule would probably be more complicated than it's worth. (There are + * probably other strange ways to call the wrong version of `captureException`, and this rule doesn't catch those, + * either, but again, unlikely to come up in real life.) + */ + +// let hasPrinted = false; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Enforce internal usage of `captureException` from `@sentry/core`', + }, + messages: { + errorMessage: + 'All internal uses of `captureException` should come directly from `@sentry/core`, not from the browser or node SDKs (or any of their wrappers), nor from the results of a `getCurrentHub()` or `getClient()` call. (The browser and node versions of `captureException`, as well as the versions living in the `Hub` and `BaseClient` classes, have manual-capture `mechanism` data baked in, which is probably not what you want.)', + }, + }, + + create: function (context) { + return { + // This catches imports of the form `import { captureException } from '@sentry/xxx';` + ImportDeclaration: function (node) { + if ( + node.specifiers.some( + specifier => + specifier.type === 'ImportSpecifier' && + specifier.imported.type === 'Identifier' && + specifier.imported.name === 'captureException', + ) && + node.source.value !== '@sentry/core' + ) { + context.report({ node, messageId: 'errorMessage' }); + } + }, + + // Places where we're calling `captureException()` + CallExpression: function (node) { + // Bare `captureException` calls, where `captureException` was imported from somewhere other than `@sentry/core` + if (node.callee.type === 'Identifier' && node.callee.name === 'captureException') { + // const captureExceptionVariable = context + // .getScope() + // .variables.find(variable => variable.name === 'captureException'); + + const captureExceptionDefinitions = getDefinitions('captureException', context); + + const captureExceptionDefinitionFromImport = + captureExceptionDefinitions && captureExceptionDefinitions.find(def => def.type === 'ImportBinding'); + + // const definitionAsImport = captureExceptionVariable.defs.find(def => def.type === 'ImportBinding'); + // const captureExceptionDefinitionFromImport = getDefinitions('captureException', context).find( + // def => def.type === 'ImportBinding', + // ); + + if ( + captureExceptionDefinitionFromImport && + captureExceptionDefinitionFromImport.parent.source.value !== '@sentry/core' + ) { + context.report({ node, messageId: 'errorMessage' }); + } + } + + // Calls of the form `someName.captureException()`, where `someName` was imported from somewhere other than `@sentry/core` + if ( + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && // the `someName` part of `someName.captureException` + node.callee.property.name === 'captureException' + ) { + const objectDefinitions = getDefinitions(node.callee.object.name, context); + + const objectDefinitionFromImport = + objectDefinitions && objectDefinitions.find(def => def.type === 'ImportBinding'); + + if (objectDefinitionFromImport && objectDefinitionFromImport.parent.source.value !== '@sentry/core') { + context.report({ node, messageId: 'errorMessage' }); + } + } + + // Calls of the form `someName.captureException()`, where `someName` is something other than an import (likely + // comething like `hub.captureException()` or `client.captureException()`) + if ( + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && // the `someName` part of `someName.captureException` + node.callee.property.name === 'captureException' + ) { + const objectDefinitions = getDefinitions(node.callee.object.name, context); + + const objectDefinitionFromImport = + objectDefinitions && objectDefinitions.find(def => def.type === 'ImportBinding'); + + if (!objectDefinitionFromImport) { + context.report({ node, messageId: 'errorMessage' }); + } + } + + // Calls of the form `.captureException()` + if ( + node.callee.type === 'MemberExpression' && + node.callee.object.type !== 'Identifier' && // the `someName` part of `someName.captureException` + node.callee.property.name === 'captureException' + ) { + context.report({ node, messageId: 'errorMessage' }); + } + + // // This checks for `import * as SomeName from '@sentry/core'; SomeName.captureException(...);`. Anytyhing else (things like `getCurrentHub().captureException(...)` and `getCurrentHub().getClient().captureException(...)` will not match and will trigger the error.) + // if (!isCoreDotCaptureException(node, context)) { + // context.report({ node, messageId: 'errorMessage' }); + // } + }, + }; + }, +}; + +// // This matches uses like `import * as SomeName from '@sentry/core'; SomeName.captureException(...);`. (We catch `import { captureException } from '@sentry/core';` elsewhere.) +// function isCoreDotCaptureException(node, context) { +// // Note: In an expression like `xxx.captureException()`: +// // - The whole thing is a `CallExpression` +// // - The `xxx.captureException` is a `MemberExpression` +// // - The `xxx` is the `object` +// // - The `captureException` is the `property` +// if ( +// node.type === 'CallExpression' && +// node.callee.type === 'MemberExpression' && +// node.callee.object.type === 'Identifier' && +// node.callee.property.name === 'captureException' +// ) { +// const objectName = node.callee.object.name; +// +// // All statements defining the object. (Not entirely clear how there there could be more than one, but +// // ¯\_(ツ)_/¯. Note: When we find a reference to the object, it may or may not be the reference in +// // `Sentry.captureException`, but we don't care, because we just want to use it to jump back to the original +// // definition.) +// const objectDefinitions = context +// .getScope() +// .references.find(reference => reference.identifier && reference.identifier.name === objectName).resolved.defs; +// +// // Of the definitions, one which comes as part of an import, if any +// const namespaceImportDef = objectDefinitions.find(definition => definition.type === 'ImportBinding'); +// +// if ( +// namespaceImportDef && +// namespaceImportDef.parent.type === 'ImportDeclaration' && +// namespaceImportDef.parent.source.value === '@sentry/core' +// ) { +// return true; +// } +// } +// +// return false; +// } + +// Get nodes defining the variable with the given name in the given scope (scope is retrieved from `context`) +function getDefinitions(name, context) { + // This is any reference to the name in the current scope, not necessarily the current one, but they must all lead back to the same definition, so it doesn't matter. + const referenceToName = context + .getScope() + .references.find(reference => reference.identifier && reference.identifier.name === name); + + return referenceToName && referenceToName.resolved && referenceToName.resolved.defs; +} + +// // Get nodes which define things with the given name +// function getDefinitions(name, context) { +// // This is all references to anything in the scope with the given name +// const matchingReferences = context +// .getScope() +// .references.filter(reference => reference.identifier && reference.identifier.name === name); +// +// return matchingReferences && matchingReferences.map(ref => ref.resolved.defs); +// } +// +// function isModuleObject(node, context) { +// if (node.type === 'Identifier') { +// // This is the name of the object. IOW, it's the `Sentry` in `Sentry.captureException`. +// const objectName = node.callee.object.name; +// +// // All statements defining the object. (Not entirely clear how there there could be more than one, but +// // ¯\_(ツ)_/¯. Note: When we find a reference to the object, it may or may not be the reference in +// // `Sentry.captureException`, but we don't care, because we just want to use it to jump back to the original +// // definition.) +// const objectDefinitions = context +// .getScope() +// .references.find(reference => reference.identifier && reference.identifier.name === objectName).resolved.defs; +// +// // Of the definitions, one which comes as part of an import, if any +// const namespaceImportDef = objectDefinitions.find(definition => definition.type === 'ImportBinding'); +// +// return namespaceImportDef !== undefined; +// } +// } diff --git a/packages/eslint-plugin-sdk/test/lib/rules/captureException-from-core.js b/packages/eslint-plugin-sdk/test/lib/rules/captureException-from-core.js new file mode 100644 index 000000000000..6e6bc9d07124 --- /dev/null +++ b/packages/eslint-plugin-sdk/test/lib/rules/captureException-from-core.js @@ -0,0 +1,97 @@ +const { RuleTester } = require('eslint'); + +const captureExceptionFromCoreRule = require('../../../src/rules/captureException-from-core.js'); +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2015, + }, +}); +ruleTester.run('captureException-from-core', captureExceptionFromCoreRule, { + valid: [ + // Solo import + "import { captureException } from '@sentry/core';", + // Solo import and call + "import { captureException, other } from '@sentry/core'; captureException('');", + // One import among many + "import { captureException, Hub } from '@sentry/core';", + // One import among many and call + "import { captureException, Hub } from '@sentry/core'; captureException('');", + // Full module import + "import * as SentryCore from '@sentry/core'; SentryCore.captureException('');", + // Full module import used inside a function + "import * as SentryCore from '@sentry/core'; const func = () => SentryCore.captureException('');", + ], + + invalid: [ + // Solo import from browser SDK + { + code: "import { captureException } from '@sentry/browser';", + errors: [{ messageId: 'errorMessage' }], + }, + // Solo import from browser SDK and call + { + code: "import { captureException } from '@sentry/browser'; captureException('');", + // Both halves of the code get flagged, so we have to anticipate two errors + errors: [{ messageId: 'errorMessage' }, { messageId: 'errorMessage' }], + }, + // Solo import from node SDK + { + code: "import { captureException } from '@sentry/node';", + errors: [{ messageId: 'errorMessage' }], + }, + // Solo import from node SDK and call + { + code: "import { captureException } from '@sentry/node'; captureException('');", + // Both halves of the code get flagged, so we have to anticipate two errors + errors: [{ messageId: 'errorMessage' }, { messageId: 'errorMessage' }], + }, + // Solo import from wrapper SDK + { + code: "import { captureException } from '@sentry/nextjs';", + errors: [{ messageId: 'errorMessage' }], + }, + // Solo import from wrapper SDK and call + { + code: "import { captureException } from '@sentry/nextjs'; captureException('');", + // Both halves of the code get flagged, so we have to anticipate two errors + errors: [{ messageId: 'errorMessage' }, { messageId: 'errorMessage' }], + }, + // One import among many, from a non-core SDK + { + code: "import { captureException, showReportDialog } from '@sentry/browser';", + errors: [{ messageId: 'errorMessage' }], + }, + // One import among many, from a non-core SDK and call + { + code: "import { captureException, showReportDialog } from '@sentry/browser'; captureException('')", + // Both halves of the code get flagged, so we have to anticipate two errors + errors: [{ messageId: 'errorMessage' }, { messageId: 'errorMessage' }], + }, + // Full module import, from a non-core SDK + { + code: "import * as SentryBrowser from '@sentry/browser'; SentryBrowser.captureException('');", + errors: [{ messageId: 'errorMessage' }], + }, + // Called on `getCurrentHub` call + { + code: "import { getCurrentHub } from '@sentry/core'; getCurrentHub().captureException('');", + errors: [{ messageId: 'errorMessage' }], + }, + // Called on `getClient` call + { + code: "import { getCurrentHub } from '@sentry/core'; getCurrentHub().getClient().captureException('');", + errors: [{ messageId: 'errorMessage' }], + }, + // Called on `hub` object` + { + code: "import { getCurrentHub } from '@sentry/core'; const hub = getCurrentHub(); hub.captureException('');", + errors: [{ messageId: 'errorMessage' }], + }, + // Called on `client` object + { + code: "import { getCurrentHub } from '@sentry/core'; const client = getCurrentHub().getClient(); client.captureException('');", + errors: [{ messageId: 'errorMessage' }], + }, + ], +}); From c80900809109475fc28e96f392af9006d548014e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:31:19 -0800 Subject: [PATCH 02/13] create protected `_captureException` method on `BaseClient` class --- packages/core/src/baseclient.ts | 44 +++++++++++++++++------------ packages/core/test/lib/base.test.ts | 3 +- packages/node/src/client.ts | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3d0a90faefbc..f0549f89ccc2 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -121,23 +121,7 @@ export abstract class BaseClient implements Client { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined { - // ensure we haven't captured this very object before - if (checkOrSetAlreadyCaught(exception)) { - __DEBUG_BUILD__ && logger.log(ALREADY_SEEN_ERROR); - return; - } - - let eventId: string | undefined = hint && hint.event_id; - - this._process( - this.eventFromException(exception, hint) - .then(event => this._captureEvent(event, hint, scope)) - .then(result => { - eventId = result; - }), - ); - - return eventId; + return this._captureException(exception, hint, scope); } /** @@ -579,6 +563,30 @@ export abstract class BaseClient implements Client { } } + /** + * Internal implementation of public `captureException` method + */ + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any + protected _captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined { + // ensure we haven't captured this very object before + if (checkOrSetAlreadyCaught(exception)) { + __DEBUG_BUILD__ && logger.log(ALREADY_SEEN_ERROR); + return; + } + + let eventId: string | undefined = hint && hint.event_id; + + this._process( + this.eventFromException(exception, hint) + .then(event => this._captureEvent(event, hint, scope)) + .then(result => { + eventId = result; + }), + ); + + return eventId; + } + /** * Processes the event and logs an error in case of rejection * @param event @@ -699,7 +707,7 @@ export abstract class BaseClient implements Client { throw reason; } - this.captureException(reason, { + this._captureException(reason, { data: { __sentry__: true, }, diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 99d6e324f9ca..56ba21c080dd 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1411,7 +1411,8 @@ describe('BaseClient', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); - const captureExceptionSpy = jest.spyOn(client, 'captureException'); + // @ts-ignore Need to spy on protected method + const captureExceptionSpy = jest.spyOn(client, '_captureException'); const loggerWarnSpy = jest.spyOn(logger, 'warn'); const scope = new Scope(); const exception = new Error('sorry'); diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 66dc8fb4ce27..c93f7dc242cf 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -60,7 +60,7 @@ export class NodeClient extends BaseClient { } } - return super.captureException(exception, hint, scope); + return super._captureException(exception, hint, scope); } /** From d5724aa73b4681b5d2780438e17d60681b90ddc8 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:42:43 -0800 Subject: [PATCH 03/13] add `captureExceptionWithHint` method to core package --- packages/core/src/exports.ts | 13 +++++++++++++ packages/core/src/index.ts | 1 + 2 files changed, 14 insertions(+) diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 97a70d327b8a..844e7e928b37 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -35,6 +35,19 @@ export function captureException(exception: any, captureContext?: CaptureContext return getCurrentHub().captureException(exception, { captureContext }); } +/** + * A version of `captureException` which takes a hint as its second parameter, primarily for internal use. + * + * @param exception An exception-like object + * @param hint Metadata for use in capturing event + * @returns The generated event id + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export function captureExceptionWithHint(exception: any, hint: EventHint): ReturnType { + // eslint-disable-next-line @sentry-internal/sdk/captureException-from-core -- This is the one place this is allowed + return getCurrentHub().captureException(exception, hint); +} + /** * Captures a message event and sends it to Sentry. * diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ad1948ce84fb..22f45e152afe 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -4,6 +4,7 @@ export type { Carrier, Layer } from './hub'; export { addBreadcrumb, captureException, + captureExceptionWithHint, captureEvent, captureMessage, configureScope, From b0b6d2557b6e9472553d8323e06287a64c2f950c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:46:21 -0800 Subject: [PATCH 04/13] use `captureExceptionWithHint` in node global handlers --- packages/node/src/integrations/onuncaughtexception.ts | 4 ++-- packages/node/src/integrations/onunhandledrejection.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index 2108e2829f63..e667d900f53e 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -1,4 +1,4 @@ -import { getCurrentHub, Scope } from '@sentry/core'; +import { captureExceptionWithHint, getCurrentHub, Scope } from '@sentry/core'; import { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -124,7 +124,7 @@ export class OnUncaughtException implements Integration { if (hub.getIntegration(OnUncaughtException)) { hub.withScope((scope: Scope) => { scope.setLevel('fatal'); - hub.captureException(error, { + captureExceptionWithHint(error, { originalException: error, data: { mechanism: { handled: false, type: 'onuncaughtexception' } }, }); diff --git a/packages/node/src/integrations/onunhandledrejection.ts b/packages/node/src/integrations/onunhandledrejection.ts index 2ba9a3d8f205..f4cd20fff504 100644 --- a/packages/node/src/integrations/onunhandledrejection.ts +++ b/packages/node/src/integrations/onunhandledrejection.ts @@ -1,4 +1,4 @@ -import { getCurrentHub, Scope } from '@sentry/core'; +import { captureExceptionWithHint, getCurrentHub, Scope } from '@sentry/core'; import { Integration } from '@sentry/types'; import { consoleSandbox } from '@sentry/utils'; @@ -49,7 +49,7 @@ export class OnUnhandledRejection implements Integration { if (hub.getIntegration(OnUnhandledRejection)) { hub.withScope((scope: Scope) => { scope.setExtra('unhandledPromiseRejection', true); - hub.captureException(reason, { + captureExceptionWithHint(reason, { originalException: promise, data: { mechanism: { handled: false, type: 'onunhandledrejection' } }, }); From 053ba9ebc489e96947a095832c431ac30cb1a2bb Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 03:52:31 -0800 Subject: [PATCH 05/13] switch angular to use `captureException` from `@sentry/core` --- packages/angular/ng-package.json | 3 ++- packages/angular/package.json | 1 + packages/angular/src/errorhandler.ts | 4 +++- packages/angular/test/errorhandler.test.ts | 9 +++++---- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/angular/ng-package.json b/packages/angular/ng-package.json index 88df70c1c7bd..3e79a0885ad9 100644 --- a/packages/angular/ng-package.json +++ b/packages/angular/ng-package.json @@ -5,9 +5,10 @@ "entryFile": "src/index.ts", "umdModuleIds": { "@sentry/browser": "Sentry", + "@sentry/core": "Sentry.core", "@sentry/utils": "Sentry.util" } }, - "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"], + "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"], "assets": ["README.md", "LICENSE"] } diff --git a/packages/angular/package.json b/packages/angular/package.json index 2b1d9be9803c..63bea592753d 100644 --- a/packages/angular/package.json +++ b/packages/angular/package.json @@ -22,6 +22,7 @@ }, "dependencies": { "@sentry/browser": "7.24.2", + "@sentry/core": "7.24.2", "@sentry/types": "7.24.2", "@sentry/utils": "7.24.2", "tslib": "^2.0.0" diff --git a/packages/angular/src/errorhandler.ts b/packages/angular/src/errorhandler.ts index a7089e0ea966..a3a07479b7f5 100644 --- a/packages/angular/src/errorhandler.ts +++ b/packages/angular/src/errorhandler.ts @@ -1,7 +1,7 @@ import { HttpErrorResponse } from '@angular/common/http'; import { ErrorHandler as AngularErrorHandler, Inject, Injectable } from '@angular/core'; import * as Sentry from '@sentry/browser'; -import { captureException } from '@sentry/browser'; +import { captureException } from '@sentry/core'; import { addExceptionMechanism } from '@sentry/utils'; import { runOutsideAngular } from './zone'; @@ -55,8 +55,10 @@ class SentryErrorHandler implements AngularErrorHandler { captureException(extractedError, scope => { scope.addEventProcessor(event => { addExceptionMechanism(event, { + // TODO: Should this be 'instrument'? type: 'angular', handled: false, + // will default to `caughtByUser: false` }); return event; diff --git a/packages/angular/test/errorhandler.test.ts b/packages/angular/test/errorhandler.test.ts index 324cbd0d1c7b..dfb399241f99 100644 --- a/packages/angular/test/errorhandler.test.ts +++ b/packages/angular/test/errorhandler.test.ts @@ -1,14 +1,15 @@ import { HttpErrorResponse } from '@angular/common/http'; -import * as SentryBrowser from '@sentry/browser'; import { Scope } from '@sentry/browser'; +import * as SentryBrowser from '@sentry/browser'; +import * as SentryCore from '@sentry/core'; import * as SentryUtils from '@sentry/utils'; import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler'; const FakeScope = new Scope(); -jest.mock('@sentry/browser', () => { - const original = jest.requireActual('@sentry/browser'); +jest.mock('@sentry/core', () => { + const original = jest.requireActual('@sentry/core'); return { ...original, captureException: (err: unknown, cb: (arg0?: unknown) => unknown) => { @@ -18,7 +19,7 @@ jest.mock('@sentry/browser', () => { }; }); -const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); +const captureExceptionSpy = jest.spyOn(SentryCore, 'captureException'); jest.spyOn(console, 'error').mockImplementation(); From 33781559275bf425cf746a8efa0eca060e210184 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:45:04 -0800 Subject: [PATCH 06/13] switch integrations to use `captureException` from `@sentry/core` --- packages/integrations/package.json | 1 + packages/integrations/src/captureconsole.ts | 3 ++- packages/integrations/test/captureconsole.test.ts | 14 ++++++++------ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/integrations/package.json b/packages/integrations/package.json index f97dca08f2ab..ea2a6067312d 100644 --- a/packages/integrations/package.json +++ b/packages/integrations/package.json @@ -16,6 +16,7 @@ "module": "build/npm/esm/index.js", "types": "build/npm/types/index.d.ts", "dependencies": { + "@sentry/core": "7.24.2", "@sentry/types": "7.24.2", "@sentry/utils": "7.24.2", "localforage": "^1.8.1", diff --git a/packages/integrations/src/captureconsole.ts b/packages/integrations/src/captureconsole.ts index ea9f2df40837..86211ed68875 100644 --- a/packages/integrations/src/captureconsole.ts +++ b/packages/integrations/src/captureconsole.ts @@ -1,3 +1,4 @@ +import { captureException } from '@sentry/core'; import { EventProcessor, Hub, Integration } from '@sentry/types'; import { CONSOLE_LEVELS, fill, GLOBAL_OBJ, safeJoin, severityLevelFromString } from '@sentry/utils'; @@ -62,7 +63,7 @@ export class CaptureConsole implements Integration { hub.captureMessage(message); } } else if (level === 'error' && args[0] instanceof Error) { - hub.captureException(args[0]); + captureException(args[0]); } else { hub.captureMessage(message); } diff --git a/packages/integrations/test/captureconsole.test.ts b/packages/integrations/test/captureconsole.test.ts index 34b715cfc7a1..43b787b07673 100644 --- a/packages/integrations/test/captureconsole.test.ts +++ b/packages/integrations/test/captureconsole.test.ts @@ -1,8 +1,11 @@ /* eslint-disable @typescript-eslint/unbound-method */ +import * as SentryCore from '@sentry/core'; import { Event, Hub, Integration } from '@sentry/types'; import { CaptureConsole } from '../src/captureconsole'; +jest.spyOn(SentryCore, 'captureException'); + const mockScope = { setLevel: jest.fn(), setExtra: jest.fn(), @@ -14,7 +17,6 @@ const mockHub = { callback(mockScope); }), captureMessage: jest.fn(), - captureException: jest.fn(), }; const mockConsole = { @@ -225,8 +227,8 @@ describe('CaptureConsole setup', () => { const someError = new Error('some error'); global.console.error(someError); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); - expect(mockHub.captureException).toHaveBeenCalledWith(someError); + expect(SentryCore.captureException).toHaveBeenCalledTimes(1); + expect(SentryCore.captureException).toHaveBeenCalledWith(someError); }); it('should capture exception on `console.error` when no levels are provided in constructor', () => { @@ -239,8 +241,8 @@ describe('CaptureConsole setup', () => { const someError = new Error('some error'); global.console.error(someError); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); - expect(mockHub.captureException).toHaveBeenCalledWith(someError); + expect(SentryCore.captureException).toHaveBeenCalledTimes(1); + expect(SentryCore.captureException).toHaveBeenCalledWith(someError); }); it('should capture message on `console.log` when no levels are provided in constructor', () => { @@ -267,7 +269,7 @@ describe('CaptureConsole setup', () => { expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); expect(mockHub.captureMessage).toHaveBeenCalledWith('some non-error message'); - expect(mockHub.captureException).not.toHaveBeenCalled(); + expect(SentryCore.captureException).not.toHaveBeenCalled(); }); it('should capture a message for non-error log levels', () => { From 202ff38e3c3fa4c68eef4f3a3e84d26ac6e5e1d6 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 05:15:24 -0800 Subject: [PATCH 07/13] switch nextjs to use `captureException` from `@sentry/core` --- packages/nextjs/src/config/wrappers/withSentryAPI.ts | 3 ++- packages/nextjs/src/utils/instrumentServer.ts | 3 ++- packages/nextjs/test/config/withSentry.test.ts | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryAPI.ts b/packages/nextjs/src/config/wrappers/withSentryAPI.ts index 72ad6356dda1..529027d3f82b 100644 --- a/packages/nextjs/src/config/wrappers/withSentryAPI.ts +++ b/packages/nextjs/src/config/wrappers/withSentryAPI.ts @@ -1,4 +1,5 @@ -import { captureException, getCurrentHub, startTransaction } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { getCurrentHub, startTransaction } from '@sentry/node'; import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing'; import { addExceptionMechanism, diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index 30967d5c28fb..77523117c496 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines */ -import { captureException, configureScope, deepReadDirSync, getCurrentHub, startTransaction } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { configureScope, deepReadDirSync, getCurrentHub, startTransaction } from '@sentry/node'; import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { addExceptionMechanism, diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts index 4314d1e32faa..60c6b1b033a2 100644 --- a/packages/nextjs/test/config/withSentry.test.ts +++ b/packages/nextjs/test/config/withSentry.test.ts @@ -38,7 +38,7 @@ async function callWrappedHandler(wrappedHandler: WrappedNextApiHandler, req: Ne // We mock `captureException` as a no-op because under normal circumstances it is an un-awaited effectively-async // function which might or might not finish before any given test ends, potentially leading jest to error out. -const captureExceptionSpy = jest.spyOn(Sentry, 'captureException').mockImplementation(jest.fn()); +const captureExceptionSpy = jest.spyOn(hub, 'captureException').mockImplementation(jest.fn()); const loggerSpy = jest.spyOn(utils.logger, 'log'); const flushSpy = jest.spyOn(Sentry, 'flush').mockImplementation(async () => { // simulate the time it takes time to flush all events From 544d0eea39d60f8c8b9c4c509cbe00d610a79147 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 05:24:21 -0800 Subject: [PATCH 08/13] switch react to use `captureException` from `@sentry/core` --- packages/react/package.json | 1 + packages/react/src/errorboundary.tsx | 3 ++- packages/react/test/errorboundary.test.tsx | 11 +++++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/react/package.json b/packages/react/package.json index d6e3a0c44507..07b0f932cc2d 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -17,6 +17,7 @@ }, "dependencies": { "@sentry/browser": "7.24.2", + "@sentry/core": "7.24.2", "@sentry/types": "7.24.2", "@sentry/utils": "7.24.2", "hoist-non-react-statics": "^3.3.2", diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx index 7aae1fbf6d08..6ef335c883ea 100644 --- a/packages/react/src/errorboundary.tsx +++ b/packages/react/src/errorboundary.tsx @@ -1,4 +1,5 @@ -import { captureException, ReportDialogOptions, Scope, showReportDialog, withScope } from '@sentry/browser'; +import { ReportDialogOptions, Scope, showReportDialog, withScope } from '@sentry/browser'; +import { captureException } from '@sentry/core'; import { isError, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import * as React from 'react'; diff --git a/packages/react/test/errorboundary.test.tsx b/packages/react/test/errorboundary.test.tsx index 7b0c25dc311a..b5281f00e25e 100644 --- a/packages/react/test/errorboundary.test.tsx +++ b/packages/react/test/errorboundary.test.tsx @@ -15,14 +15,21 @@ const mockCaptureException = jest.fn(); const mockShowReportDialog = jest.fn(); const EVENT_ID = 'test-id-123'; -jest.mock('@sentry/browser', () => { - const actual = jest.requireActual('@sentry/browser'); +jest.mock('@sentry/core', () => { + const actual = jest.requireActual('@sentry/core'); return { ...actual, captureException: (...args: unknown[]) => { mockCaptureException(...args); return EVENT_ID; }, + }; +}); + +jest.mock('@sentry/browser', () => { + const actual = jest.requireActual('@sentry/browser'); + return { + ...actual, showReportDialog: (options: any) => { mockShowReportDialog(options); }, From a6bde18eaf52bb1d42db9b6276b166d510502a3b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 05:26:30 -0800 Subject: [PATCH 09/13] switch remix to use `captureException` from `@sentry/core` --- packages/remix/src/utils/instrumentServer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 4aee67a174b4..8b39bc8fdb61 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines */ -import { captureException, getCurrentHub, Hub } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { getCurrentHub, Hub } from '@sentry/node'; import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; import { From e39b7bdec2547498b5f9c9da3381fae62d97795f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 05:38:16 -0800 Subject: [PATCH 10/13] switch serverless to use `captureException` from `@sentry/core` --- packages/serverless/package.json | 1 + packages/serverless/src/awslambda.ts | 3 +- .../src/gcpfunction/cloud_events.ts | 3 +- packages/serverless/src/gcpfunction/events.ts | 3 +- packages/serverless/src/gcpfunction/http.ts | 3 +- .../serverless/test/__mocks__/@sentry/core.ts | 5 ++++ .../serverless/test/__mocks__/@sentry/node.ts | 28 ------------------- packages/serverless/test/awslambda.test.ts | 20 ++++++------- packages/serverless/test/awsservices.test.ts | 3 +- packages/serverless/test/gcpfunction.test.ts | 20 ++++++------- .../serverless/test/google-cloud-grpc.test.ts | 7 ++--- .../serverless/test/google-cloud-http.test.ts | 3 +- 12 files changed, 38 insertions(+), 61 deletions(-) create mode 100644 packages/serverless/test/__mocks__/@sentry/core.ts diff --git a/packages/serverless/package.json b/packages/serverless/package.json index fb8604bffd50..2564d7e50b0f 100644 --- a/packages/serverless/package.json +++ b/packages/serverless/package.json @@ -16,6 +16,7 @@ "access": "public" }, "dependencies": { + "@sentry/core": "7.24.2", "@sentry/node": "7.24.2", "@sentry/tracing": "7.24.2", "@sentry/types": "7.24.2", diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 18b9ea2387d4..0d015f24dab9 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines */ +import { captureException } from '@sentry/core'; import * as Sentry from '@sentry/node'; -import { captureException, captureMessage, flush, getCurrentHub, Scope, withScope } from '@sentry/node'; +import { captureMessage, flush, getCurrentHub, Scope, withScope } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; import { baggageHeaderToDynamicSamplingContext, dsnFromString, dsnToString, isString, logger } from '@sentry/utils'; diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts index 6ab7b173784c..4d31cfb4c3e4 100644 --- a/packages/serverless/src/gcpfunction/cloud_events.ts +++ b/packages/serverless/src/gcpfunction/cloud_events.ts @@ -1,4 +1,5 @@ -import { captureException, flush, getCurrentHub } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { flush, getCurrentHub } from '@sentry/node'; import { logger } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from '../utils'; diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts index 6b83fd8bfba0..72c4181e33e3 100644 --- a/packages/serverless/src/gcpfunction/events.ts +++ b/packages/serverless/src/gcpfunction/events.ts @@ -1,4 +1,5 @@ -import { captureException, flush, getCurrentHub } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { flush, getCurrentHub } from '@sentry/node'; import { logger } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from '../utils'; diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 3b62d8301bf1..ffe84d393f0e 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -1,4 +1,5 @@ -import { AddRequestDataToEventOptions, captureException, flush, getCurrentHub } from '@sentry/node'; +import { captureException } from '@sentry/core'; +import { AddRequestDataToEventOptions, flush, getCurrentHub } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; import { baggageHeaderToDynamicSamplingContext, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; diff --git a/packages/serverless/test/__mocks__/@sentry/core.ts b/packages/serverless/test/__mocks__/@sentry/core.ts new file mode 100644 index 000000000000..1438013ceb7b --- /dev/null +++ b/packages/serverless/test/__mocks__/@sentry/core.ts @@ -0,0 +1,5 @@ +const origSentry = jest.requireActual('@sentry/core'); +export const BaseClient = origSentry.BaseClient; +export const Integrations = origSentry.Integrations; +export const getMainCarrier = origSentry.getMainCarrier; +export const captureException = jest.fn(); diff --git a/packages/serverless/test/__mocks__/@sentry/node.ts b/packages/serverless/test/__mocks__/@sentry/node.ts index 355065317ea7..710b34e90f38 100644 --- a/packages/serverless/test/__mocks__/@sentry/node.ts +++ b/packages/serverless/test/__mocks__/@sentry/node.ts @@ -35,34 +35,6 @@ export const init = jest.fn(); export const addGlobalEventProcessor = jest.fn(); export const getCurrentHub = jest.fn(() => fakeHub); export const startTransaction = jest.fn(_ => fakeTransaction); -export const captureException = jest.fn(); export const captureMessage = jest.fn(); export const withScope = jest.fn(cb => cb(fakeScope)); export const flush = jest.fn(() => Promise.resolve()); - -export const resetMocks = (): void => { - fakeTransaction.setHttpStatus.mockClear(); - fakeTransaction.finish.mockClear(); - fakeTransaction.startChild.mockClear(); - fakeSpan.finish.mockClear(); - fakeHub.configureScope.mockClear(); - fakeHub.pushScope.mockClear(); - fakeHub.popScope.mockClear(); - fakeHub.getScope.mockClear(); - - fakeScope.addEventProcessor.mockClear(); - fakeScope.setTransactionName.mockClear(); - fakeScope.setTag.mockClear(); - fakeScope.setContext.mockClear(); - fakeScope.setSpan.mockClear(); - fakeScope.getTransaction.mockClear(); - - init.mockClear(); - addGlobalEventProcessor.mockClear(); - getCurrentHub.mockClear(); - startTransaction.mockClear(); - captureException.mockClear(); - captureMessage.mockClear(); - withScope.mockClear(); - flush.mockClear(); -}; diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index 419974e73bfe..6626d9ac8f6c 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -1,3 +1,4 @@ +import * as SentryCore from '@sentry/core'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import { Callback, Handler } from 'aws-lambda'; @@ -76,8 +77,7 @@ describe('AWSLambda', () => { }); afterEach(() => { - // @ts-ignore see "Why @ts-ignore" note - Sentry.resetMocks(); + jest.clearAllMocks(); }); describe('wrapHandler() options', () => { @@ -159,7 +159,7 @@ describe('AWSLambda', () => { const handler = () => Promise.resolve([{ status: 'rejected', reason: new Error() }]); const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337 }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(Sentry.captureException).toBeCalledTimes(0); + expect(SentryCore.captureException).toBeCalledTimes(0); }); test('captureAllSettledReasons enable', async () => { @@ -173,9 +173,9 @@ describe('AWSLambda', () => { ]); const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337, captureAllSettledReasons: true }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(Sentry.captureException).toHaveBeenNthCalledWith(1, error); - expect(Sentry.captureException).toHaveBeenNthCalledWith(2, error2); - expect(Sentry.captureException).toBeCalledTimes(2); + expect(SentryCore.captureException).toHaveBeenNthCalledWith(1, error); + expect(SentryCore.captureException).toHaveBeenNthCalledWith(2, error2); + expect(SentryCore.captureException).toBeCalledTimes(2); }); }); @@ -225,7 +225,7 @@ describe('AWSLambda', () => { // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); expectScopeSettings(fakeTransactionContext); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalledWith(2000); @@ -302,7 +302,7 @@ describe('AWSLambda', () => { // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); expectScopeSettings(fakeTransactionContext); - expect(Sentry.captureException).toBeCalledWith(e); + expect(SentryCore.captureException).toBeCalledWith(e); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -367,7 +367,7 @@ describe('AWSLambda', () => { // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); expectScopeSettings(fakeTransactionContext); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -447,7 +447,7 @@ describe('AWSLambda', () => { // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); expectScopeSettings(fakeTransactionContext); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); diff --git a/packages/serverless/test/awsservices.test.ts b/packages/serverless/test/awsservices.test.ts index fd1be7a74689..3759be9fd1b2 100644 --- a/packages/serverless/test/awsservices.test.ts +++ b/packages/serverless/test/awsservices.test.ts @@ -16,8 +16,7 @@ describe('AWSServices', () => { new AWSServices().setupOnce(); }); afterEach(() => { - // @ts-ignore see "Why @ts-ignore" note - Sentry.resetMocks(); + jest.clearAllMocks(); }); afterAll(() => { nock.restore(); diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 20f878565a56..f2f09c16fda3 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -1,3 +1,4 @@ +import * as SentryCore from '@sentry/core'; import * as SentryNode from '@sentry/node'; import * as domain from 'domain'; @@ -22,8 +23,7 @@ import { describe('GCPFunction', () => { afterEach(() => { - // @ts-ignore see "Why @ts-ignore" note - Sentry.resetMocks(); + jest.clearAllMocks(); }); async function handleHttp(fn: HttpFunction, trace_headers: { [key: string]: string } | null = null): Promise { @@ -195,7 +195,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -312,7 +312,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -375,7 +375,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -431,7 +431,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -459,7 +459,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); }); }); @@ -525,7 +525,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -581,7 +581,7 @@ describe('GCPFunction', () => { expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); @@ -610,7 +610,7 @@ describe('GCPFunction', () => { // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction); - expect(Sentry.captureException).toBeCalledWith(error); + expect(SentryCore.captureException).toBeCalledWith(error); }); }); diff --git a/packages/serverless/test/google-cloud-grpc.test.ts b/packages/serverless/test/google-cloud-grpc.test.ts index ffd0d0144346..8a1cb5fcab70 100644 --- a/packages/serverless/test/google-cloud-grpc.test.ts +++ b/packages/serverless/test/google-cloud-grpc.test.ts @@ -85,9 +85,7 @@ describe('GoogleCloudGrpc tracing', () => { nock('https://www.googleapis.com').post('/oauth2/v4/token').reply(200, []); }); afterEach(() => { - // @ts-ignore see "Why @ts-ignore" note - Sentry.resetMocks(); - spyConnect.mockClear(); + jest.clearAllMocks(); }); afterAll(() => { nock.restore(); @@ -118,8 +116,7 @@ describe('GoogleCloudGrpc tracing', () => { }); afterEach(() => { - dnsLookup.mockReset(); - resolveTxt.mockReset(); + jest.resetAllMocks(); }); test('publish', async () => { diff --git a/packages/serverless/test/google-cloud-http.test.ts b/packages/serverless/test/google-cloud-http.test.ts index 7a34d5026a5b..e3e0f0253333 100644 --- a/packages/serverless/test/google-cloud-http.test.ts +++ b/packages/serverless/test/google-cloud-http.test.ts @@ -23,8 +23,7 @@ describe('GoogleCloudHttp tracing', () => { .reply(200, '{"access_token":"a.b.c","expires_in":3599,"token_type":"Bearer"}'); }); afterEach(() => { - // @ts-ignore see "Why @ts-ignore" note - Sentry.resetMocks(); + jest.clearAllMocks(); }); afterAll(() => { nock.restore(); From c0cd8fdd873e84aa4e04b812376c0e2d3fb3c681 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Dec 2022 05:40:23 -0800 Subject: [PATCH 11/13] switch vue to use `captureException` from `@sentry/core` --- packages/vue/src/errorhandler.ts | 3 ++- packages/vue/src/router.ts | 3 ++- packages/vue/test/router.test.ts | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/vue/src/errorhandler.ts b/packages/vue/src/errorhandler.ts index 16cb09a723ab..926cd8b8f35c 100644 --- a/packages/vue/src/errorhandler.ts +++ b/packages/vue/src/errorhandler.ts @@ -1,4 +1,5 @@ import { getCurrentHub } from '@sentry/browser'; +import { captureException } from '@sentry/core'; import { formatComponentName, generateComponentTrace } from './components'; import { Options, ViewModel, Vue } from './types'; @@ -31,7 +32,7 @@ export const attachErrorHandler = (app: Vue, options: Options): void => { setTimeout(() => { getCurrentHub().withScope(scope => { scope.setContext('vue', metadata); - getCurrentHub().captureException(error); + captureException(error); }); }); diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index f782246e8fb2..e5954c9f1453 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -1,4 +1,5 @@ -import { captureException, WINDOW } from '@sentry/browser'; +import { WINDOW } from '@sentry/browser'; +import { captureException } from '@sentry/core'; import { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getActiveTransaction } from './tracing'; diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index c4072616d9af..a54af7cd319f 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -1,11 +1,11 @@ -import * as SentryBrowser from '@sentry/browser'; +import * as SentryCore from '@sentry/core'; import { Transaction } from '@sentry/types'; import { vueRouterInstrumentation } from '../src'; import { Route } from '../src/router'; import * as vueTracing from '../src/tracing'; -const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); +const captureExceptionSpy = jest.spyOn(SentryCore, 'captureException'); const mockVueRouter = { onError: jest.fn void]>(), From 3fc6eb2c9e5d46403d55606c6e4f45a9dcab0418 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:42:02 -0800 Subject: [PATCH 12/13] disable eslint rule in core's own `captureException` function --- packages/core/src/exports.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 844e7e928b37..39050ee30434 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -32,6 +32,7 @@ import { Scope } from './scope'; */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export function captureException(exception: any, captureContext?: CaptureContext): ReturnType { + // eslint-disable-next-line @sentry-internal/sdk/captureException-from-core -- This is the one place this is allowed return getCurrentHub().captureException(exception, { captureContext }); } From 38ba255bfdd51e3491ad5294666be9f7bf3837c5 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Sun, 11 Dec 2022 22:43:55 -0800 Subject: [PATCH 13/13] disable eslint rule for hub's call to client `captureException` --- packages/core/src/hub.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 5036158e08eb..52fe08d2282c 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -187,6 +187,7 @@ export class Hub implements HubInterface { const eventId = (this._lastEventId = hint && hint.event_id ? hint.event_id : uuid4()); const syntheticException = new Error('Sentry syntheticException'); this._withClient((client, scope) => { + // eslint-disable-next-line @sentry-internal/sdk/captureException-from-core client.captureException( exception, {