Skip to content

ref(build): Add debug constants in each package individually #4842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ If you run into trouble writing tests and need to debug one of them, you can do

Pro tip: If any of your breakpoints are in code run by multiple tests, and you run the whole test file, you'll land on those breakpoints over and over again, in the middle of tests you don't care about. To avoid this, replace the test's initial `it` or `test` with `it.only` or `test.only`. That way, when you hit a breakpoint, you'll know you got there are part of the buggy test.

## Debug Build Flags

Throughout the codebase, you will find debug flags like `IS_DEBUG_BUILD` guarding various code sections.
These flags serve two purposes:

1. They enable us to remove debug code for our production browser bundles.
2. Enable users to tree-shake Sentry debug code for their production builds.

These debug flags need to be declared in each package individually and must not be imported across package boundaries, because some build tools have trouble tree-shaking imported guards.
As a convention, we define debug flags in a `flags.ts` file in the root of a package's `src` folder.
The `flags.ts` file will contain "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during our, or the user's build process.
Take care when introducing new flags - they must not throw if they are not replaced.

## Linting

Similar to building and testing, linting can be done in the project root or in individual packages by calling `yarn lint`.
Expand Down
18 changes: 18 additions & 0 deletions packages/angular/src/flags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* This file defines flags and constants that can be modified during compile time in order to facilitate tree shaking
* for users.
*
* Debug flags need to be declared in each package individually and must not be imported across package boundaries,
* because some build tools have trouble tree-shaking imported guards.
*
* As a convention, we define debug flags in a `flags.ts` file in the root of a package's `src` folder.
*
* Debug flag files will contain "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during
* our, or the user's build process. Take care when introducing new flags - they must not throw if they are not
* replaced.
*/

declare const __SENTRY_DEBUG__: boolean;

/** Flag that is true for debug builds, false otherwise. */
export const IS_DEBUG_BUILD = typeof __SENTRY_DEBUG__ === 'undefined' ? true : __SENTRY_DEBUG__;
5 changes: 3 additions & 2 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni
import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
import { getCurrentHub } from '@sentry/browser';
import { Span, Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, isDebugBuild, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
import { Observable, Subscription } from 'rxjs';
import { filter, tap } from 'rxjs/operators';

import { ANGULAR_INIT_OP, ANGULAR_OP, ANGULAR_ROUTING_OP } from './constants';
import { IS_DEBUG_BUILD } from './flags';
import { runOutsideAngular } from './zone';

let instrumentationInitialized: boolean;
Expand Down Expand Up @@ -63,7 +64,7 @@ export class TraceService implements OnDestroy {
filter(event => event instanceof NavigationStart),
tap(event => {
if (!instrumentationInitialized) {
isDebugBuild() &&
IS_DEBUG_BUILD &&
logger.error('Angular integration has tracing enabled, but Tracing integration is not configured');
return;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
import { Event, EventHint } from '@sentry/types';
import { getGlobalObject, isDebugBuild, logger } from '@sentry/utils';
import { getGlobalObject, logger } from '@sentry/utils';

import { BrowserBackend, BrowserOptions } from './backend';
import { IS_DEBUG_BUILD } from './flags';
import { injectReportDialog, ReportDialogOptions } from './helpers';
import { Breadcrumbs } from './integrations';

Expand Down Expand Up @@ -47,7 +48,7 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
}

if (!this._isEnabled()) {
isDebugBuild() && logger.error('Trying to call showReportDialog with Sentry Client disabled');
IS_DEBUG_BUILD && logger.error('Trying to call showReportDialog with Sentry Client disabled');
return;
}

Expand Down
18 changes: 18 additions & 0 deletions packages/browser/src/flags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* This file defines flags and constants that can be modified during compile time in order to facilitate tree shaking
* for users.
*
* Debug flags need to be declared in each package individually and must not be imported across package boundaries,
* because some build tools have trouble tree-shaking imported guards.
*
* As a convention, we define debug flags in a `flags.ts` file in the root of a package's `src` folder.
*
* Debug flag files will contain "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during
* our, or the user's build process. Take care when introducing new flags - they must not throw if they are not
* replaced.
*/

declare const __SENTRY_DEBUG__: boolean;

/** Flag that is true for debug builds, false otherwise. */
export const IS_DEBUG_BUILD = typeof __SENTRY_DEBUG__ === 'undefined' ? true : __SENTRY_DEBUG__;
7 changes: 4 additions & 3 deletions packages/browser/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import {
addNonEnumerableProperty,
getGlobalObject,
getOriginalFunction,
isDebugBuild,
logger,
markFunctionWrapped,
} from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';

const global = getGlobalObject<Window>();
let ignoreOnError: number = 0;

Expand Down Expand Up @@ -192,12 +193,12 @@ export function injectReportDialog(options: ReportDialogOptions = {}): void {
}

if (!options.eventId) {
isDebugBuild() && logger.error('Missing eventId option in showReportDialog call');
IS_DEBUG_BUILD && logger.error('Missing eventId option in showReportDialog call');
return;
}

if (!options.dsn) {
isDebugBuild() && logger.error('Missing dsn option in showReportDialog call');
IS_DEBUG_BUILD && logger.error('Missing dsn option in showReportDialog call');
return;
}

Expand Down
6 changes: 4 additions & 2 deletions packages/browser/src/integrations/dedupe.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';
import { isDebugBuild, logger } from '@sentry/utils';
import { logger } from '@sentry/utils';

import { IS_DEBUG_BUILD } from '../flags';

/** Deduplication filter */
export class Dedupe implements Integration {
Expand Down Expand Up @@ -28,7 +30,7 @@ export class Dedupe implements Integration {
// Juuust in case something goes wrong
try {
if (_shouldDropEvent(currentEvent, self._previousEvent)) {
isDebugBuild() && logger.warn('Event dropped due to being a duplicate of previously captured event.');
IS_DEBUG_BUILD && logger.warn('Event dropped due to being a duplicate of previously captured event.');
return null;
}
} catch (_oO) {
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import {
addExceptionMechanism,
addInstrumentationHandler,
getLocationHref,
isDebugBuild,
isErrorEvent,
isPrimitive,
isString,
logger,
} from '@sentry/utils';

import { eventFromUnknownInput } from '../eventbuilder';
import { IS_DEBUG_BUILD } from '../flags';
import { shouldIgnoreOnError } from '../helpers';

type GlobalHandlersIntegrationsOptionKeys = 'onerror' | 'onunhandledrejection';
Expand Down Expand Up @@ -237,7 +237,7 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column
}

function globalHandlerLog(type: string): void {
isDebugBuild() && logger.log(`Global Handler attached: ${type}`);
IS_DEBUG_BUILD && logger.log(`Global Handler attached: ${type}`);
}

function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {
Expand Down
9 changes: 5 additions & 4 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
import { Hub } from '@sentry/types';
import { addInstrumentationHandler, getGlobalObject, isDebugBuild, logger, resolvedSyncPromise } from '@sentry/utils';
import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils';

import { BrowserOptions } from './backend';
import { BrowserClient } from './client';
import { IS_DEBUG_BUILD } from './flags';
import { ReportDialogOptions, wrap as internalWrap } from './helpers';
import { Breadcrumbs, Dedupe, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations';

Expand Down Expand Up @@ -162,7 +163,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
if (client) {
return client.flush(timeout);
}
isDebugBuild() && logger.warn('Cannot flush events. No client defined.');
IS_DEBUG_BUILD && logger.warn('Cannot flush events. No client defined.');
return resolvedSyncPromise(false);
}

Expand All @@ -179,7 +180,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
if (client) {
return client.close(timeout);
}
isDebugBuild() && logger.warn('Cannot flush events and disable SDK. No client defined.');
IS_DEBUG_BUILD && logger.warn('Cannot flush events and disable SDK. No client defined.');
return resolvedSyncPromise(false);
}

Expand Down Expand Up @@ -208,7 +209,7 @@ function startSessionTracking(): void {
const document = window.document;

if (typeof document === 'undefined') {
isDebugBuild() && logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
IS_DEBUG_BUILD && logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
return;
}

Expand Down
12 changes: 6 additions & 6 deletions packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
dsnToString,
eventStatusFromHttpCode,
getGlobalObject,
isDebugBuild,
isRateLimited,
logger,
makePromiseBuffer,
Expand All @@ -33,6 +32,7 @@ import {
updateRateLimits,
} from '@sentry/utils';

import { IS_DEBUG_BUILD } from '../flags';
import { sendReport } from './utils';

function requestTypeToCategory(ty: SentryRequestType): string {
Expand Down Expand Up @@ -108,7 +108,7 @@ export abstract class BaseTransport implements Transport {
// A correct type for map-based implementation if we want to go that route
// would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
const key = `${requestTypeToCategory(category)}:${reason}`;
isDebugBuild() && logger.log(`Adding outcome: ${key}`);
IS_DEBUG_BUILD && logger.log(`Adding outcome: ${key}`);
this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
}

Expand All @@ -125,11 +125,11 @@ export abstract class BaseTransport implements Transport {

// Nothing to send
if (!Object.keys(outcomes).length) {
isDebugBuild() && logger.log('No outcomes to flush');
IS_DEBUG_BUILD && logger.log('No outcomes to flush');
return;
}

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

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

Expand All @@ -147,7 +147,7 @@ export abstract class BaseTransport implements Transport {
try {
sendReport(url, serializeEnvelope(envelope));
} catch (e) {
isDebugBuild() && logger.error(e);
IS_DEBUG_BUILD && logger.error(e);
}
}

Expand All @@ -172,7 +172,7 @@ export abstract class BaseTransport implements Transport {
this._rateLimits = updateRateLimits(this._rateLimits, headers);
// eslint-disable-next-line deprecation/deprecation
if (this._isRateLimited(requestType)) {
isDebugBuild() &&
IS_DEBUG_BUILD &&
// eslint-disable-next-line deprecation/deprecation
logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
}
Expand Down
6 changes: 4 additions & 2 deletions packages/browser/src/transports/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { forget, getGlobalObject, isDebugBuild, isNativeFetch, logger, supportsFetch } from '@sentry/utils';
import { forget, getGlobalObject, isNativeFetch, logger, supportsFetch } from '@sentry/utils';

import { IS_DEBUG_BUILD } from '../flags';

const global = getGlobalObject<Window>();
let cachedFetchImpl: FetchImpl;
Expand Down Expand Up @@ -69,7 +71,7 @@ export function getNativeFetchImplementation(): FetchImpl {
}
document.head.removeChild(sandbox);
} catch (e) {
isDebugBuild() &&
IS_DEBUG_BUILD &&
logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e);
}
}
Expand Down
15 changes: 8 additions & 7 deletions packages/core/src/basebackend.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Event, EventHint, Options, Session, Severity, Transport } from '@sentry/types';
import { isDebugBuild, logger, SentryError } from '@sentry/utils';
import { logger, SentryError } from '@sentry/utils';

import { initAPIDetails } from './api';
import { IS_DEBUG_BUILD } from './flags';
import { createEventEnvelope, createSessionEnvelope } from './request';
import { NewTransport } from './transports/base';
import { NoopTransport } from './transports/noop';
Expand Down Expand Up @@ -73,7 +74,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
public constructor(options: O) {
this._options = options;
if (!this._options.dsn) {
isDebugBuild() && logger.warn('No DSN provided, backend will not do anything.');
IS_DEBUG_BUILD && logger.warn('No DSN provided, backend will not do anything.');
}
this._transport = this._setupTransport();
}
Expand Down Expand Up @@ -107,11 +108,11 @@ export abstract class BaseBackend<O extends Options> implements Backend {
const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel);
const env = createEventEnvelope(event, api);
void this._newTransport.send(env).then(null, reason => {
isDebugBuild() && logger.error('Error while sending event:', reason);
IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
});
} else {
void this._transport.sendEvent(event).then(null, reason => {
isDebugBuild() && logger.error('Error while sending event:', reason);
IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
});
}
}
Expand All @@ -121,7 +122,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
*/
public sendSession(session: Session): void {
if (!this._transport.sendSession) {
isDebugBuild() && logger.warn("Dropping session because custom transport doesn't implement sendSession");
IS_DEBUG_BUILD && logger.warn("Dropping session because custom transport doesn't implement sendSession");
return;
}

Expand All @@ -135,11 +136,11 @@ export abstract class BaseBackend<O extends Options> implements Backend {
const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel);
const [env] = createSessionEnvelope(session, api);
void this._newTransport.send(env).then(null, reason => {
isDebugBuild() && logger.error('Error while sending session:', reason);
IS_DEBUG_BUILD && logger.error('Error while sending session:', reason);
});
} else {
void this._transport.sendSession(session).then(null, reason => {
isDebugBuild() && logger.error('Error while sending session:', reason);
IS_DEBUG_BUILD && logger.error('Error while sending session:', reason);
});
}
}
Expand Down
Loading