Skip to content

Commit 2d3b861

Browse files
lobsterkatielobsterkatie
authored
feat(various): Apply debug guard to logger everywhere (#4698)
This applies the `isDebugBuild()` guard (which allows logging code to be stripped when creating minified bundles) to a number of aspects of the logger: - All calls to the logger now have the guard. While it's true that not all log statements appear in spots which currently have any chance of making it into a bundle, applying the guard universally reduces cognitive load for both folks reading and writing our code, and future-proofs (or, perhaps more accurately, new-bundling-situation-proofs) all of our logging. - The `Sentry.init()` option `debug` now only enables the logger if we're in a debug build. This allows us to remove any reference to the logger in non-debug builds, making it able to be dropped from the code. A warning message has also been added to non-debug builds, because someone using `debug` is presumably doing so because they actually do want logging. - The `logger` singleton is only attached to `global.__SENTRY__` if we're in a debug build. This makes `logger.ts` side-effect-free in non-debug builds, allowing it to be treeshaken freely. Note that because the return value of `isDebugBuild()` is always `true` (absent intervention by a bundler), it's safe to use everywhere without changing any current behavior. Finally, all preexisting `if`-block versions of the check are converted to short-circuit versions, for consistency and concision.
1 parent b664554 commit 2d3b861

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+279
-251
lines changed

‎packages/angular/src/tracing.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni
22
import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
33
import { getCurrentHub } from '@sentry/browser';
44
import { Span, Transaction, TransactionContext } from '@sentry/types';
5-
import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
5+
import { getGlobalObject, isDebugBuild, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
66
import { Observable, Subscription } from 'rxjs';
77
import { filter, tap } from 'rxjs/operators';
88

@@ -63,7 +63,8 @@ export class TraceService implements OnDestroy {
6363
filter(event => event instanceof NavigationStart),
6464
tap(event => {
6565
if (!instrumentationInitialized) {
66-
logger.error('Angular integration has tracing enabled, but Tracing integration is not configured');
66+
isDebugBuild() &&
67+
logger.error('Angular integration has tracing enabled, but Tracing integration is not configured');
6768
return;
6869
}
6970

‎packages/browser/src/client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
22
import { Event, EventHint } from '@sentry/types';
3-
import { getGlobalObject, logger } from '@sentry/utils';
3+
import { getGlobalObject, isDebugBuild, logger } from '@sentry/utils';
44

55
import { BrowserBackend, BrowserOptions } from './backend';
66
import { injectReportDialog, ReportDialogOptions } from './helpers';
@@ -47,7 +47,7 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
4747
}
4848

4949
if (!this._isEnabled()) {
50-
logger.error('Trying to call showReportDialog with Sentry Client disabled');
50+
isDebugBuild() && logger.error('Trying to call showReportDialog with Sentry Client disabled');
5151
return;
5252
}
5353

‎packages/browser/src/helpers.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,12 @@ export function injectReportDialog(options: ReportDialogOptions = {}): void {
192192
}
193193

194194
if (!options.eventId) {
195-
if (isDebugBuild()) {
196-
logger.error('Missing eventId option in showReportDialog call');
197-
}
195+
isDebugBuild() && logger.error('Missing eventId option in showReportDialog call');
198196
return;
199197
}
200198

201199
if (!options.dsn) {
202-
if (isDebugBuild()) {
203-
logger.error('Missing dsn option in showReportDialog call');
204-
}
200+
isDebugBuild() && logger.error('Missing dsn option in showReportDialog call');
205201
return;
206202
}
207203

‎packages/browser/src/integrations/dedupe.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';
2-
import { logger } from '@sentry/utils';
2+
import { isDebugBuild, logger } from '@sentry/utils';
33

44
/** Deduplication filter */
55
export class Dedupe implements Integration {
@@ -28,7 +28,7 @@ export class Dedupe implements Integration {
2828
// Juuust in case something goes wrong
2929
try {
3030
if (_shouldDropEvent(currentEvent, self._previousEvent)) {
31-
logger.warn('Event dropped due to being a duplicate of previously captured event.');
31+
isDebugBuild() && logger.warn('Event dropped due to being a duplicate of previously captured event.');
3232
return null;
3333
}
3434
} catch (_oO) {

‎packages/browser/src/integrations/globalhandlers.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,7 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column
237237
}
238238

239239
function globalHandlerLog(type: string): void {
240-
if (isDebugBuild()) {
241-
logger.log(`Global Handler attached: ${type}`);
242-
}
240+
isDebugBuild() && logger.log(`Global Handler attached: ${type}`);
243241
}
244242

245243
function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {

‎packages/browser/src/sdk.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
162162
if (client) {
163163
return client.flush(timeout);
164164
}
165-
if (isDebugBuild()) {
166-
logger.warn('Cannot flush events. No client defined.');
167-
}
165+
isDebugBuild() && logger.warn('Cannot flush events. No client defined.');
168166
return resolvedSyncPromise(false);
169167
}
170168

@@ -181,9 +179,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
181179
if (client) {
182180
return client.close(timeout);
183181
}
184-
if (isDebugBuild()) {
185-
logger.warn('Cannot flush events and disable SDK. No client defined.');
186-
}
182+
isDebugBuild() && logger.warn('Cannot flush events and disable SDK. No client defined.');
187183
return resolvedSyncPromise(false);
188184
}
189185

@@ -212,9 +208,7 @@ function startSessionTracking(): void {
212208
const document = window.document;
213209

214210
if (typeof document === 'undefined') {
215-
if (isDebugBuild()) {
216-
logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
217-
}
211+
isDebugBuild() && logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
218212
return;
219213
}
220214

‎packages/browser/src/transports/base.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export abstract class BaseTransport implements Transport {
105105
// A correct type for map-based implementation if we want to go that route
106106
// would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
107107
const key = `${requestTypeToCategory(category)}:${reason}`;
108-
logger.log(`Adding outcome: ${key}`);
108+
isDebugBuild() && logger.log(`Adding outcome: ${key}`);
109109
this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
110110
}
111111

@@ -122,11 +122,11 @@ export abstract class BaseTransport implements Transport {
122122

123123
// Nothing to send
124124
if (!Object.keys(outcomes).length) {
125-
logger.log('No outcomes to flush');
125+
isDebugBuild() && logger.log('No outcomes to flush');
126126
return;
127127
}
128128

129-
logger.log(`Flushing outcomes:\n${JSON.stringify(outcomes, null, 2)}`);
129+
isDebugBuild() && logger.log(`Flushing outcomes:\n${JSON.stringify(outcomes, null, 2)}`);
130130

131131
const url = getEnvelopeEndpointWithUrlEncodedAuth(this._api.dsn, this._api.tunnel);
132132

@@ -144,7 +144,7 @@ export abstract class BaseTransport implements Transport {
144144
try {
145145
sendReport(url, serializeEnvelope(envelope));
146146
} catch (e) {
147-
logger.error(e);
147+
isDebugBuild() && logger.error(e);
148148
}
149149
}
150150

@@ -170,8 +170,9 @@ export abstract class BaseTransport implements Transport {
170170
* https://developer.mozilla.org/en-US/docs/Web/API/Headers/get
171171
*/
172172
const limited = this._handleRateLimit(headers);
173-
if (limited && isDebugBuild()) {
174-
logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
173+
if (limited) {
174+
isDebugBuild() &&
175+
logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
175176
}
176177

177178
if (status === 'success') {

‎packages/browser/src/transports/utils.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,8 @@ export function getNativeFetchImplementation(): FetchImpl {
6969
}
7070
document.head.removeChild(sandbox);
7171
} catch (e) {
72-
if (isDebugBuild()) {
72+
isDebugBuild() &&
7373
logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e);
74-
}
7574
}
7675
}
7776

‎packages/core/src/basebackend.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
6767
public constructor(options: O) {
6868
this._options = options;
6969
if (!this._options.dsn) {
70-
logger.warn('No DSN provided, backend will not do anything.');
70+
isDebugBuild() && logger.warn('No DSN provided, backend will not do anything.');
7171
}
7272
this._transport = this._setupTransport();
7373
}
@@ -92,9 +92,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
9292
*/
9393
public sendEvent(event: Event): void {
9494
void this._transport.sendEvent(event).then(null, reason => {
95-
if (isDebugBuild()) {
96-
logger.error('Error while sending event:', reason);
97-
}
95+
isDebugBuild() && logger.error('Error while sending event:', reason);
9896
});
9997
}
10098

@@ -103,16 +101,12 @@ export abstract class BaseBackend<O extends Options> implements Backend {
103101
*/
104102
public sendSession(session: Session): void {
105103
if (!this._transport.sendSession) {
106-
if (isDebugBuild()) {
107-
logger.warn("Dropping session because custom transport doesn't implement sendSession");
108-
}
104+
isDebugBuild() && logger.warn("Dropping session because custom transport doesn't implement sendSession");
109105
return;
110106
}
111107

112108
void this._transport.sendSession(session).then(null, reason => {
113-
if (isDebugBuild()) {
114-
logger.error('Error while sending session:', reason);
115-
}
109+
isDebugBuild() && logger.error('Error while sending session:', reason);
116110
});
117111
}
118112

‎packages/core/src/baseclient.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
108108
public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
109109
// ensure we haven't captured this very object before
110110
if (checkOrSetAlreadyCaught(exception)) {
111-
logger.log(ALREADY_SEEN_ERROR);
111+
isDebugBuild() && logger.log(ALREADY_SEEN_ERROR);
112112
return;
113113
}
114114

@@ -153,7 +153,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
153153
public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
154154
// ensure we haven't captured this very object before
155155
if (hint && hint.originalException && checkOrSetAlreadyCaught(hint.originalException)) {
156-
logger.log(ALREADY_SEEN_ERROR);
156+
isDebugBuild() && logger.log(ALREADY_SEEN_ERROR);
157157
return;
158158
}
159159

@@ -173,16 +173,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
173173
*/
174174
public captureSession(session: Session): void {
175175
if (!this._isEnabled()) {
176-
if (isDebugBuild()) {
177-
logger.warn('SDK not enabled, will not capture session.');
178-
}
176+
isDebugBuild() && logger.warn('SDK not enabled, will not capture session.');
179177
return;
180178
}
181179

182180
if (!(typeof session.release === 'string')) {
183-
if (isDebugBuild()) {
184-
logger.warn('Discarded session because of missing or non-string release');
185-
}
181+
isDebugBuild() && logger.warn('Discarded session because of missing or non-string release');
186182
} else {
187183
this._sendSession(session);
188184
// After sending, we set init false to indicate it's not the first occurrence
@@ -248,7 +244,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
248244
try {
249245
return (this._integrations[integration.id] as T) || null;
250246
} catch (_oO) {
251-
logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
247+
isDebugBuild() && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
252248
return null;
253249
}
254250
}
@@ -507,7 +503,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
507503
return finalEvent.event_id;
508504
},
509505
reason => {
510-
logger.error(reason);
506+
isDebugBuild() && logger.error(reason);
511507
return undefined;
512508
},
513509
);

‎packages/core/src/integration.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
22
import { Integration, Options } from '@sentry/types';
3-
import { addNonEnumerableProperty, logger } from '@sentry/utils';
3+
import { addNonEnumerableProperty, isDebugBuild, logger } from '@sentry/utils';
44

55
export const installedIntegrations: string[] = [];
66

@@ -59,7 +59,7 @@ export function setupIntegration(integration: Integration): void {
5959
}
6060
integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
6161
installedIntegrations.push(integration.name);
62-
logger.log(`Integration installed: ${integration.name}`);
62+
isDebugBuild() && logger.log(`Integration installed: ${integration.name}`);
6363
}
6464

6565
/**

‎packages/core/src/integrations/inboundfilters.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,37 +64,33 @@ export class InboundFilters implements Integration {
6464
/** JSDoc */
6565
private _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
6666
if (this._isSentryError(event, options)) {
67-
if (isDebugBuild()) {
67+
isDebugBuild() &&
6868
logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);
69-
}
7069
return true;
7170
}
7271
if (this._isIgnoredError(event, options)) {
73-
if (isDebugBuild()) {
72+
isDebugBuild() &&
7473
logger.warn(
7574
`Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`,
7675
);
77-
}
7876
return true;
7977
}
8078
if (this._isDeniedUrl(event, options)) {
81-
if (isDebugBuild()) {
79+
isDebugBuild() &&
8280
logger.warn(
8381
`Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription(
8482
event,
8583
)}.\nUrl: ${this._getEventFilterUrl(event)}`,
8684
);
87-
}
8885
return true;
8986
}
9087
if (!this._isAllowedUrl(event, options)) {
91-
if (isDebugBuild()) {
88+
isDebugBuild() &&
9289
logger.warn(
9390
`Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription(
9491
event,
9592
)}.\nUrl: ${this._getEventFilterUrl(event)}`,
9693
);
97-
}
9894
return true;
9995
}
10096
return false;
@@ -187,9 +183,7 @@ export class InboundFilters implements Integration {
187183
const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {};
188184
return [`${value}`, `${type}: ${value}`];
189185
} catch (oO) {
190-
if (isDebugBuild()) {
191-
logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
192-
}
186+
isDebugBuild() && logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
193187
return [];
194188
}
195189
}
@@ -224,9 +218,7 @@ export class InboundFilters implements Integration {
224218
}
225219
return frames ? this._getLastValidUrl(frames) : null;
226220
} catch (oO) {
227-
if (isDebugBuild()) {
228-
logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
229-
}
221+
isDebugBuild() && logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
230222
return null;
231223
}
232224
}

‎packages/core/src/sdk.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getCurrentHub } from '@sentry/hub';
22
import { Client, Options } from '@sentry/types';
3-
import { logger } from '@sentry/utils';
3+
import { isDebugBuild, logger } from '@sentry/utils';
44

55
/** A class object that can instantiate Client objects. */
66
export type ClientClass<F extends Client, O extends Options> = new (options: O) => F;
@@ -14,7 +14,13 @@ export type ClientClass<F extends Client, O extends Options> = new (options: O)
1414
*/
1515
export function initAndBind<F extends Client, O extends Options>(clientClass: ClientClass<F, O>, options: O): void {
1616
if (options.debug === true) {
17-
logger.enable();
17+
if (isDebugBuild()) {
18+
logger.enable();
19+
} else {
20+
// use `console.warn` rather than `logger.warn` since by non-debug bundles have all `logger.x` statements stripped
21+
// eslint-disable-next-line no-console
22+
console.warn('[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.');
23+
}
1824
}
1925
const hub = getCurrentHub();
2026
const scope = hub.getScope();

0 commit comments

Comments
 (0)