From 031e091e93a70b7117fe4d8ee5818e492747bdbc Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 15 Aug 2022 21:14:21 -0700 Subject: [PATCH 1/4] add `ConsoleLevel` type --- packages/utils/src/logger.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index 8fc9c0976fd0..b7111a216871 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -9,6 +9,7 @@ const global = getGlobalObject(); const PREFIX = 'Sentry Logger '; export const CONSOLE_LEVELS = ['debug', 'info', 'warn', 'error', 'log', 'assert', 'trace'] as const; +export type ConsoleLevel = typeof CONSOLE_LEVELS[number]; type LoggerMethod = (...args: unknown[]) => void; type LoggerConsoleMethods = Record; From ea2fa959f7fac30def749577bd1f978090aa7fab Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 10 Aug 2022 14:57:34 -0700 Subject: [PATCH 2/4] suppress stack when `SentryError` isn't an error --- packages/core/src/baseclient.ts | 18 ++++++++++++++---- packages/utils/src/error.ts | 7 ++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 6518a7feec0f..6d246106048c 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -583,7 +583,16 @@ export abstract class BaseClient implements Client { return finalEvent.event_id; }, reason => { - __DEBUG_BUILD__ && logger.warn(reason); + if (__DEBUG_BUILD__) { + // If something's gone wrong, log the error as a warning. If it's just us having used a `SentryError` for + // control flow, log just the message (no stack) as a log-level log. + const sentryError = reason as SentryError; + if (sentryError.logLevel === 'log') { + logger.log(sentryError.message); + } else { + logger.warn(sentryError); + } + } return undefined; }, ); @@ -606,7 +615,7 @@ export abstract class BaseClient implements Client { const { beforeSend, sampleRate } = this.getOptions(); if (!this._isEnabled()) { - return rejectedSyncPromise(new SentryError('SDK not enabled, will not capture event.')); + return rejectedSyncPromise(new SentryError('SDK not enabled, will not capture event.', 'log')); } const isTransaction = event.type === 'transaction'; @@ -618,6 +627,7 @@ export abstract class BaseClient implements Client { return rejectedSyncPromise( new SentryError( `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, + 'log', ), ); } @@ -626,7 +636,7 @@ export abstract class BaseClient implements Client { .then(prepared => { if (prepared === null) { this.recordDroppedEvent('event_processor', event.type || 'error'); - throw new SentryError('An event processor returned null, will not send event.'); + throw new SentryError('An event processor returned null, will not send event.', 'log'); } const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true; @@ -640,7 +650,7 @@ export abstract class BaseClient implements Client { .then(processedEvent => { if (processedEvent === null) { this.recordDroppedEvent('before_send', event.type || 'error'); - throw new SentryError('`beforeSend` returned `null`, will not send event.'); + throw new SentryError('`beforeSend` returned `null`, will not send event.', 'log'); } const session = scope && scope.getSession(); diff --git a/packages/utils/src/error.ts b/packages/utils/src/error.ts index 30b35c115d14..3015d08e779c 100644 --- a/packages/utils/src/error.ts +++ b/packages/utils/src/error.ts @@ -1,12 +1,17 @@ +import type { ConsoleLevel } from './logger'; + /** An error emitted by Sentry SDKs and related utilities. */ export class SentryError extends Error { /** Display name of this error instance. */ public name: string; - public constructor(public message: string) { + public logLevel: ConsoleLevel; + + public constructor(public message: string, logLevel: ConsoleLevel = 'warn') { super(message); this.name = new.target.prototype.constructor.name; Object.setPrototypeOf(this, new.target.prototype); + this.logLevel = logLevel; } } From 7a4e8668eacad9c171f71c6415f26af958cc8d98 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 10 Aug 2022 16:19:26 -0700 Subject: [PATCH 3/4] fix tests --- packages/core/test/lib/base.test.ts | 8 ++++---- packages/nextjs/test/index.client.test.ts | 6 +++--- packages/nextjs/test/index.server.test.ts | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 742a9f0a7ae3..9c3fcd86f27f 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -920,13 +920,13 @@ describe('BaseClient', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); const client = new TestClient(options); const captureExceptionSpy = jest.spyOn(client, 'captureException'); - const loggerWarnSpy = jest.spyOn(logger, 'warn'); + const loggerWarnSpy = jest.spyOn(logger, 'log'); client.captureEvent({ message: 'hello' }); expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toBeCalled(); - expect(loggerWarnSpy).toBeCalledWith(new SentryError('`beforeSend` returned `null`, will not send event.')); + expect(loggerWarnSpy).toBeCalledWith('`beforeSend` returned `null`, will not send event.'); }); test('calls beforeSend and log info about invalid return value', () => { @@ -1065,7 +1065,7 @@ describe('BaseClient', () => { const client = new TestClient(getDefaultTestClientOptions({ dsn: PUBLIC_DSN })); const captureExceptionSpy = jest.spyOn(client, 'captureException'); - const loggerWarnSpy = jest.spyOn(logger, 'warn'); + const loggerLogSpy = jest.spyOn(logger, 'log'); const scope = new Scope(); scope.addEventProcessor(() => null); @@ -1073,7 +1073,7 @@ describe('BaseClient', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toBeCalled(); - expect(loggerWarnSpy).toBeCalledWith(new SentryError('An event processor returned null, will not send event.')); + expect(loggerLogSpy).toBeCalledWith('An event processor returned null, will not send event.'); }); test('eventProcessor records dropped events', () => { diff --git a/packages/nextjs/test/index.client.test.ts b/packages/nextjs/test/index.client.test.ts index a143591ad8f0..a4f2972537be 100644 --- a/packages/nextjs/test/index.client.test.ts +++ b/packages/nextjs/test/index.client.test.ts @@ -3,7 +3,7 @@ import { getCurrentHub } from '@sentry/hub'; import * as SentryReact from '@sentry/react'; import { Integrations as TracingIntegrations } from '@sentry/tracing'; import { Integration } from '@sentry/types'; -import { getGlobalObject, logger, SentryError } from '@sentry/utils'; +import { getGlobalObject, logger } from '@sentry/utils'; import { init, Integrations, nextRouterInstrumentation } from '../src/index.client'; import { NextjsOptions } from '../src/utils/nextjsOptions'; @@ -14,7 +14,7 @@ const global = getGlobalObject(); const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); -const logWarn = jest.spyOn(logger, 'warn'); +const loggerLogSpy = jest.spyOn(logger, 'log'); describe('Client init()', () => { afterEach(() => { @@ -75,7 +75,7 @@ describe('Client init()', () => { expect(transportSend).not.toHaveBeenCalled(); expect(captureEvent.mock.results[0].value).toBeUndefined(); - expect(logWarn).toHaveBeenCalledWith(new SentryError('An event processor returned null, will not send event.')); + expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned null, will not send event.'); }); describe('integrations', () => { diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/index.server.test.ts index d926d8bbd57e..c1955e241054 100644 --- a/packages/nextjs/test/index.server.test.ts +++ b/packages/nextjs/test/index.server.test.ts @@ -2,7 +2,7 @@ import { RewriteFrames } from '@sentry/integrations'; import * as SentryNode from '@sentry/node'; import { getCurrentHub, NodeClient } from '@sentry/node'; import { Integration } from '@sentry/types'; -import { getGlobalObject, logger, SentryError } from '@sentry/utils'; +import { getGlobalObject, logger } from '@sentry/utils'; import * as domain from 'domain'; import { init } from '../src/index.server'; @@ -16,7 +16,7 @@ const global = getGlobalObject(); (global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next'; const nodeInit = jest.spyOn(SentryNode, 'init'); -const logWarn = jest.spyOn(logger, 'warn'); +const loggerLogSpy = jest.spyOn(logger, 'log'); describe('Server init()', () => { afterEach(() => { @@ -104,7 +104,7 @@ describe('Server init()', () => { await SentryNode.flush(); expect(transportSend).not.toHaveBeenCalled(); - expect(logWarn).toHaveBeenCalledWith(new SentryError('An event processor returned null, will not send event.')); + expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned null, will not send event.'); }); it("initializes both global hub and domain hub when there's an active domain", () => { From c56310aaac996d6f7b1d325679d82c910a873ce4 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 15 Aug 2022 23:44:23 -0700 Subject: [PATCH 4/4] add note about `SentryError` prototype --- packages/utils/src/error.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/utils/src/error.ts b/packages/utils/src/error.ts index 3015d08e779c..ae64f09a3643 100644 --- a/packages/utils/src/error.ts +++ b/packages/utils/src/error.ts @@ -11,6 +11,9 @@ export class SentryError extends Error { super(message); this.name = new.target.prototype.constructor.name; + // This sets the prototype to be `Error`, not `SentryError`. It's unclear why we do this, but commenting this line + // out causes various (seemingly totally unrelated) playwright tests consistently time out. FYI, this makes + // instances of `SentryError` fail `obj instanceof SentryError` checks. Object.setPrototypeOf(this, new.target.prototype); this.logLevel = logLevel; }