From 2f215142f9b86707041f68a0baad9f5b40c966a8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 20 Dec 2021 22:22:51 +0100 Subject: [PATCH 1/2] feat(logging): trim down debug logging further for non debug builds --- packages/browser/src/helpers.ts | 9 +++- .../src/integrations/globalhandlers.ts | 5 +- packages/browser/src/sdk.ts | 31 ++++++++----- packages/browser/src/transports/base.ts | 4 +- packages/browser/src/transports/utils.ts | 6 ++- packages/core/src/basebackend.ts | 14 ++++-- packages/core/src/baseclient.ts | 9 +++- .../core/src/integrations/inboundfilters.ts | 46 ++++++++++++------- packages/types/src/hub.ts | 2 +- packages/utils/src/instrument.ts | 13 ++++-- packages/utils/src/supports.ts | 5 +- 11 files changed, 97 insertions(+), 47 deletions(-) diff --git a/packages/browser/src/helpers.ts b/packages/browser/src/helpers.ts index 1b802d2a22de..1e4647d2d7bd 100644 --- a/packages/browser/src/helpers.ts +++ b/packages/browser/src/helpers.ts @@ -6,6 +6,7 @@ import { addNonEnumerableProperty, getGlobalObject, getOriginalFunction, + isDebugBuild, logger, markFunctionWrapped, } from '@sentry/utils'; @@ -191,12 +192,16 @@ export function injectReportDialog(options: ReportDialogOptions = {}): void { } if (!options.eventId) { - logger.error(`Missing eventId option in showReportDialog call`); + if (isDebugBuild()) { + logger.error(`Missing eventId option in showReportDialog call`); + } return; } if (!options.dsn) { - logger.error(`Missing dsn option in showReportDialog call`); + if (isDebugBuild()) { + logger.error(`Missing dsn option in showReportDialog call`); + } return; } diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index 8a3586cd6edd..e71962d567d8 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -5,6 +5,7 @@ import { addExceptionMechanism, addInstrumentationHandler, getLocationHref, + isDebugBuild, isErrorEvent, isPrimitive, isString, @@ -239,7 +240,9 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column } function globalHandlerLog(type: string): void { - logger.log(`Global Handler attached: ${type}`); + if (isDebugBuild()) { + logger.log(`Global Handler attached: ${type}`); + } } function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void { diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index c980d986d3b3..ed3f0c8ba888 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -1,5 +1,6 @@ import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core'; -import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils'; +import { Hub } from '@sentry/types'; +import { addInstrumentationHandler, getGlobalObject, isDebugBuild, logger, resolvedSyncPromise } from '@sentry/utils'; import { BrowserOptions } from './backend'; import { BrowserClient } from './client'; @@ -161,7 +162,9 @@ export function flush(timeout?: number): PromiseLike { if (client) { return client.flush(timeout); } - logger.warn('Cannot flush events. No client defined.'); + if (isDebugBuild()) { + logger.warn('Cannot flush events. No client defined.'); + } return resolvedSyncPromise(false); } @@ -178,7 +181,9 @@ export function close(timeout?: number): PromiseLike { if (client) { return client.close(timeout); } - logger.warn('Cannot flush events and disable SDK. No client defined.'); + if (isDebugBuild()) { + logger.warn('Cannot flush events and disable SDK. No client defined.'); + } return resolvedSyncPromise(false); } @@ -194,6 +199,11 @@ export function wrap(fn: (...args: any) => any): any { return internalWrap(fn)(); } +function startSessionOnHub(hub: Hub): void { + hub.startSession({ ignoreDuration: true }); + hub.captureSession(); +} + /** * Enable automatic Session Tracking for the initial page load. */ @@ -202,7 +212,9 @@ function startSessionTracking(): void { const document = window.document; if (typeof document === 'undefined') { - logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.'); + if (isDebugBuild()) { + logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.'); + } return; } @@ -214,7 +226,7 @@ function startSessionTracking(): void { // https://github.com/getsentry/sentry-javascript/issues/3207 and // https://github.com/getsentry/sentry-javascript/issues/3234 and // https://github.com/getsentry/sentry-javascript/issues/3278. - if (typeof hub.startSession !== 'function' || typeof hub.captureSession !== 'function') { + if (!hub.captureSession) { return; } @@ -222,16 +234,13 @@ function startSessionTracking(): void { // concept that can be used as a metric. // Automatically captured sessions are akin to page views, and thus we // discard their duration. - hub.startSession({ ignoreDuration: true }); - hub.captureSession(); + startSessionOnHub(hub); // We want to create a session for every navigation as well addInstrumentationHandler('history', ({ from, to }) => { // Don't create an additional session for the initial route or if the location did not change - if (from === undefined || from === to) { - return; + if (!(from === undefined || from === to)) { + startSessionOnHub(getCurrentHub()); } - hub.startSession({ ignoreDuration: true }); - hub.captureSession(); }); } diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index 042311d53694..f2db2f2807a9 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -16,6 +16,7 @@ import { dateTimestampInSeconds, eventStatusFromHttpCode, getGlobalObject, + isDebugBuild, logger, makePromiseBuffer, parseRetryAfterHeader, @@ -162,8 +163,9 @@ export abstract class BaseTransport implements Transport { * https://developer.mozilla.org/en-US/docs/Web/API/Headers/get */ const limited = this._handleRateLimit(headers); - if (limited) + if (limited && isDebugBuild()) { logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`); + } if (status === 'success') { resolve({ status }); diff --git a/packages/browser/src/transports/utils.ts b/packages/browser/src/transports/utils.ts index 5d958b531034..5b48c9e93cbc 100644 --- a/packages/browser/src/transports/utils.ts +++ b/packages/browser/src/transports/utils.ts @@ -1,4 +1,4 @@ -import { forget, getGlobalObject, isNativeFetch, logger, supportsFetch } from '@sentry/utils'; +import { forget, getGlobalObject, isDebugBuild, isNativeFetch, logger, supportsFetch } from '@sentry/utils'; const global = getGlobalObject(); let cachedFetchImpl: FetchImpl; @@ -69,7 +69,9 @@ export function getNativeFetchImplementation(): FetchImpl { } document.head.removeChild(sandbox); } catch (e) { - logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e); + if (isDebugBuild()) { + logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e); + } } } diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index ffdcd22af6c0..806c46dfcdc6 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -1,5 +1,5 @@ import { Event, EventHint, Options, Session, SeverityLevel, Transport } from '@sentry/types'; -import { logger, SentryError } from '@sentry/utils'; +import { isDebugBuild, logger, SentryError } from '@sentry/utils'; import { NoopTransport } from './transports/noop'; @@ -92,7 +92,9 @@ export abstract class BaseBackend implements Backend { */ public sendEvent(event: Event): void { void this._transport.sendEvent(event).then(null, reason => { - logger.error(`Error while sending event: ${reason}`); + if (isDebugBuild()) { + logger.error(`Error while sending event: ${reason}`); + } }); } @@ -101,12 +103,16 @@ export abstract class BaseBackend implements Backend { */ public sendSession(session: Session): void { if (!this._transport.sendSession) { - logger.warn("Dropping session because custom transport doesn't implement sendSession"); + if (isDebugBuild()) { + logger.warn("Dropping session because custom transport doesn't implement sendSession"); + } return; } void this._transport.sendSession(session).then(null, reason => { - logger.error(`Error while sending session: ${reason}`); + if (isDebugBuild()) { + logger.error(`Error while sending session: ${reason}`); + } }); } diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 8ff1bb60e739..4b4ecdb25089 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -14,6 +14,7 @@ import { checkOrSetAlreadyCaught, dateTimestampInSeconds, Dsn, + isDebugBuild, isPlainObject, isPrimitive, isThenable, @@ -171,12 +172,16 @@ export abstract class BaseClient implement */ public captureSession(session: Session): void { if (!this._isEnabled()) { - logger.warn('SDK not enabled, will not capture session.'); + if (isDebugBuild()) { + logger.warn('SDK not enabled, will not capture session.'); + } return; } if (!(typeof session.release === 'string')) { - logger.warn('Discarded session because of missing or non-string release'); + if (isDebugBuild()) { + logger.warn('Discarded session because of missing or non-string release'); + } } else { this._sendSession(session); // After sending, we set init false to indicate it's not the first occurrence diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 7bfda9e56a92..f779c3be6266 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -1,6 +1,6 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub'; import { Event, Integration, StackFrame } from '@sentry/types'; -import { getEventDescription, isMatchingPattern, logger } from '@sentry/utils'; +import { getEventDescription, isDebugBuild, isMatchingPattern, logger } from '@sentry/utils'; // "Script error." is hard coded into browsers for errors that it can't read. // this is the result of a script being pulled in from an external domain and CORS. @@ -64,29 +64,37 @@ export class InboundFilters implements Integration { /** JSDoc */ private _shouldDropEvent(event: Event, options: Partial): boolean { if (this._isSentryError(event, options)) { - logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); + if (isDebugBuild()) { + logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); + } return true; } if (this._isIgnoredError(event, options)) { - logger.warn( - `Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`, - ); + if (isDebugBuild()) { + logger.warn( + `Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`, + ); + } return true; } if (this._isDeniedUrl(event, options)) { - logger.warn( - `Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription( - event, - )}.\nUrl: ${this._getEventFilterUrl(event)}`, - ); + if (isDebugBuild()) { + logger.warn( + `Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription( + event, + )}.\nUrl: ${this._getEventFilterUrl(event)}`, + ); + } return true; } if (!this._isAllowedUrl(event, options)) { - logger.warn( - `Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription( - event, - )}.\nUrl: ${this._getEventFilterUrl(event)}`, - ); + if (isDebugBuild()) { + logger.warn( + `Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription( + event, + )}.\nUrl: ${this._getEventFilterUrl(event)}`, + ); + } return true; } return false; @@ -179,7 +187,9 @@ export class InboundFilters implements Integration { const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {}; return [`${value}`, `${type}: ${value}`]; } catch (oO) { - logger.error(`Cannot extract message for event ${getEventDescription(event)}`); + if (isDebugBuild()) { + logger.error(`Cannot extract message for event ${getEventDescription(event)}`); + } return []; } } @@ -214,7 +224,9 @@ export class InboundFilters implements Integration { } return frames ? this._getLastValidUrl(frames) : null; } catch (oO) { - logger.error(`Cannot extract url for event ${getEventDescription(event)}`); + if (isDebugBuild()) { + logger.error(`Cannot extract url for event ${getEventDescription(event)}`); + } return null; } } diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index dd3bc1d5b2ab..6dac7c47e545 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -227,5 +227,5 @@ export interface Hub { * Sends the current session on the scope to Sentry * @param endSession If set the session will be marked as exited and removed from the scope */ - captureSession(endSession: boolean): void; + captureSession(endSession?: boolean): void; } diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts index 8dd358c484e0..15e920e78b9a 100644 --- a/packages/utils/src/instrument.ts +++ b/packages/utils/src/instrument.ts @@ -2,6 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/ban-types */ import { WrappedFunction } from '@sentry/types'; +import { isDebugBuild } from '.'; import { getGlobalObject } from './global'; import { isInstanceOf, isString } from './is'; @@ -93,11 +94,13 @@ function triggerHandlers(type: InstrumentHandlerType, data: any): void { try { handler(data); } catch (e) { - logger.error( - `Error while triggering instrumentation handler.\nType: ${type}\nName: ${getFunctionName( - handler, - )}\nError: ${e}`, - ); + if (isDebugBuild()) { + logger.error( + `Error while triggering instrumentation handler.\nType: ${type}\nName: ${getFunctionName( + handler, + )}\nError: ${e}`, + ); + } } } } diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index 1a548b45f0d0..780ea088f9f3 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -1,5 +1,6 @@ import { getGlobalObject } from './global'; import { logger } from './logger'; +import { isDebugBuild } from './env'; /** * Tells whether current environment supports ErrorEvent objects @@ -112,7 +113,9 @@ export function supportsNativeFetch(): boolean { } doc.head.removeChild(sandbox); } catch (err) { - logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', err); + if (isDebugBuild()) { + logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', err); + } } } From 4ce20a93949c85978751ae59b5092cdf2fd8cdf5 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 21 Dec 2021 11:23:21 +0100 Subject: [PATCH 2/2] fix: lint --- packages/utils/src/instrument.ts | 2 +- packages/utils/src/supports.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts index 15e920e78b9a..76b7060eae88 100644 --- a/packages/utils/src/instrument.ts +++ b/packages/utils/src/instrument.ts @@ -2,8 +2,8 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/ban-types */ import { WrappedFunction } from '@sentry/types'; -import { isDebugBuild } from '.'; +import { isDebugBuild } from '.'; import { getGlobalObject } from './global'; import { isInstanceOf, isString } from './is'; import { logger } from './logger'; diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index 780ea088f9f3..baaf0d1fab1d 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -1,6 +1,6 @@ +import { isDebugBuild } from './env'; import { getGlobalObject } from './global'; import { logger } from './logger'; -import { isDebugBuild } from './env'; /** * Tells whether current environment supports ErrorEvent objects