Skip to content

Commit

Permalink
ref(dsn): trim dsn size (#4325)
Browse files Browse the repository at this point in the history
* ref(utils): dsn

* ref(utils): dsn

* ref(utils): dsn

* ref(core): simplify dsn parsing

* ref(utils): get global url

* ref(utils): fix test

* Revert "ref(utils): fix test"

This reverts commit 13cf627.

* Revert "ref(utils): get global url"

This reverts commit 61feeeb.

* Revert "ref(core): simplify dsn parsing"

This reverts commit c90b6f9.

* fix(dsn): update test

* ref(utils): drop dsn toString class method

* ref(utils): drop export

* fix import

* fix import

* fix import
  • Loading branch information
JonasBa authored Dec 21, 2021
1 parent 1c3f85a commit c36de75
Show file tree
Hide file tree
Showing 13 changed files with 201 additions and 225 deletions.
3 changes: 2 additions & 1 deletion packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '@sentry/types';
import {
dateTimestampInSeconds,
dsnToString,
eventStatusFromHttpCode,
getGlobalObject,
logger,
Expand Down Expand Up @@ -116,7 +117,7 @@ export abstract class BaseTransport implements Transport {

const url = getEnvelopeEndpointWithUrlEncodedAuth(this._api.dsn, this._api.tunnel);
// Envelope header is required to be at least an empty object
const envelopeHeader = JSON.stringify({ ...(this._api.tunnel && { dsn: this._api.dsn.toString() }) });
const envelopeHeader = JSON.stringify({ ...(this._api.tunnel && { dsn: dsnToString(this._api.dsn) }) });
const itemHeaders = JSON.stringify({
type: 'client_report',
});
Expand Down
38 changes: 21 additions & 17 deletions packages/core/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DsnLike, SdkMetadata } from '@sentry/types';
import { Dsn, urlEncode } from '@sentry/utils';
import { DsnComponents, DsnLike, SdkMetadata } from '@sentry/types';
import { dsnToString, makeDsn, urlEncode } from '@sentry/utils';

const SENTRY_API_VERSION = '7';

Expand All @@ -12,7 +12,7 @@ export interface APIDetails {
/** Metadata about the SDK (name, version, etc) for inclusion in envelope headers */
metadata: SdkMetadata;
/** The internally used Dsn object. */
readonly dsn: Dsn;
readonly dsn: DsnComponents;
/** The envelope tunnel to use. */
readonly tunnel?: string;
}
Expand All @@ -32,21 +32,21 @@ export class API {
public metadata: SdkMetadata;

/** The internally used Dsn object. */
private readonly _dsnObject: Dsn;
private readonly _dsnObject: DsnComponents;

/** The envelope tunnel to use. */
private readonly _tunnel?: string;

/** Create a new instance of API */
public constructor(dsn: DsnLike, metadata: SdkMetadata = {}, tunnel?: string) {
this.dsn = dsn;
this._dsnObject = new Dsn(dsn);
this._dsnObject = makeDsn(dsn);
this.metadata = metadata;
this._tunnel = tunnel;
}

/** Returns the Dsn object. */
public getDsn(): Dsn {
public getDsn(): DsnComponents {
return this._dsnObject;
}

Expand Down Expand Up @@ -89,25 +89,25 @@ export function initAPIDetails(dsn: DsnLike, metadata?: SdkMetadata, tunnel?: st
return {
initDsn: dsn,
metadata: metadata || {},
dsn: new Dsn(dsn),
dsn: makeDsn(dsn),
tunnel,
} as APIDetails;
}

/** Returns the prefix to construct Sentry ingestion API endpoints. */
function getBaseApiEndpoint(dsn: Dsn): string {
function getBaseApiEndpoint(dsn: DsnComponents): string {
const protocol = dsn.protocol ? `${dsn.protocol}:` : '';
const port = dsn.port ? `:${dsn.port}` : '';
return `${protocol}//${dsn.host}${port}${dsn.path ? `/${dsn.path}` : ''}/api/`;
}

/** Returns the ingest API endpoint for target. */
function _getIngestEndpoint(dsn: Dsn, target: 'store' | 'envelope'): string {
function _getIngestEndpoint(dsn: DsnComponents, target: 'store' | 'envelope'): string {
return `${getBaseApiEndpoint(dsn)}${dsn.projectId}/${target}/`;
}

/** Returns a URL-encoded string with auth config suitable for a query string. */
function _encodedAuth(dsn: Dsn): string {
function _encodedAuth(dsn: DsnComponents): string {
return urlEncode({
// We send only the minimum set of required information. See
// https://github.com/getsentry/sentry-javascript/issues/2572.
Expand All @@ -117,7 +117,7 @@ function _encodedAuth(dsn: Dsn): string {
}

/** Returns the store endpoint URL. */
function getStoreEndpoint(dsn: Dsn): string {
function getStoreEndpoint(dsn: DsnComponents): string {
return _getIngestEndpoint(dsn, 'store');
}

Expand All @@ -126,12 +126,12 @@ function getStoreEndpoint(dsn: Dsn): string {
*
* Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
*/
export function getStoreEndpointWithUrlEncodedAuth(dsn: Dsn): string {
export function getStoreEndpointWithUrlEncodedAuth(dsn: DsnComponents): string {
return `${getStoreEndpoint(dsn)}?${_encodedAuth(dsn)}`;
}

/** Returns the envelope endpoint URL. */
function _getEnvelopeEndpoint(dsn: Dsn): string {
function _getEnvelopeEndpoint(dsn: DsnComponents): string {
return _getIngestEndpoint(dsn, 'envelope');
}

Expand All @@ -140,15 +140,19 @@ function _getEnvelopeEndpoint(dsn: Dsn): string {
*
* Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
*/
export function getEnvelopeEndpointWithUrlEncodedAuth(dsn: Dsn, tunnel?: string): string {
export function getEnvelopeEndpointWithUrlEncodedAuth(dsn: DsnComponents, tunnel?: string): string {
return tunnel ? tunnel : `${_getEnvelopeEndpoint(dsn)}?${_encodedAuth(dsn)}`;
}

/**
* Returns an object that can be used in request headers.
* This is needed for node and the old /store endpoint in sentry
*/
export function getRequestHeaders(dsn: Dsn, clientName: string, clientVersion: string): { [key: string]: string } {
export function getRequestHeaders(
dsn: DsnComponents,
clientName: string,
clientVersion: string,
): { [key: string]: string } {
// CHANGE THIS to use metadata but keep clientName and clientVersion compatible
const header = [`Sentry sentry_version=${SENTRY_API_VERSION}`];
header.push(`sentry_client=${clientName}/${clientVersion}`);
Expand All @@ -171,10 +175,10 @@ export function getReportDialogEndpoint(
user?: { name?: string; email?: string };
},
): string {
const dsn = new Dsn(dsnLike);
const dsn = makeDsn(dsnLike);
const endpoint = `${getBaseApiEndpoint(dsn)}embed/error-page/`;

let encodedOptions = `dsn=${dsn.toString()}`;
let encodedOptions = `dsn=${dsnToString(dsn)}`;
for (const key in dialogOptions) {
if (key === 'dsn') {
continue;
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { Scope, Session } from '@sentry/hub';
import {
Client,
DsnComponents,
Event,
EventHint,
Integration,
Expand All @@ -13,11 +14,11 @@ import {
import {
checkOrSetAlreadyCaught,
dateTimestampInSeconds,
Dsn,
isPlainObject,
isPrimitive,
isThenable,
logger,
makeDsn,
normalize,
rejectedSyncPromise,
resolvedSyncPromise,
Expand Down Expand Up @@ -76,7 +77,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
protected readonly _options: O;

/** The client Dsn, if specified in options. Without this Dsn, the SDK will be disabled. */
protected readonly _dsn?: Dsn;
protected readonly _dsn?: DsnComponents;

/** Array of used integrations. */
protected _integrations: IntegrationIndex = {};
Expand All @@ -95,7 +96,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
this._options = options;

if (options.dsn) {
this._dsn = new Dsn(options.dsn);
this._dsn = makeDsn(options.dsn);
}
}

Expand Down Expand Up @@ -187,7 +188,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
/**
* @inheritDoc
*/
public getDsn(): Dsn | undefined {
public getDsn(): DsnComponents | undefined {
return this._dsn;
}

Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event, SdkInfo, SentryRequest, SentryRequestType, Session, SessionAggregates } from '@sentry/types';
import { dsnToString } from '@sentry/utils';

import { APIDetails, getEnvelopeEndpointWithUrlEncodedAuth, getStoreEndpointWithUrlEncodedAuth } from './api';

Expand Down Expand Up @@ -33,7 +34,7 @@ export function sessionToSentryRequest(session: Session | SessionAggregates, api
const envelopeHeaders = JSON.stringify({
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
...(!!api.tunnel && { dsn: api.dsn.toString() }),
...(!!api.tunnel && { dsn: dsnToString(api.dsn) }),
});
// I know this is hacky but we don't want to add `session` to request type since it's never rate limited
const type: SentryRequestType = 'aggregates' in session ? ('sessions' as SentryRequestType) : 'session';
Expand Down Expand Up @@ -81,7 +82,7 @@ export function eventToSentryRequest(event: Event, api: APIDetails): SentryReque
event_id: event.event_id,
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
...(!!api.tunnel && { dsn: api.dsn.toString() }),
...(!!api.tunnel && { dsn: dsnToString(api.dsn) }),
});
const itemHeaders = JSON.stringify({
type: eventType,
Expand Down
14 changes: 10 additions & 4 deletions packages/core/test/lib/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable deprecation/deprecation */
import { Dsn } from '@sentry/utils';
import { makeDsn } from '@sentry/utils';

import { API, getReportDialogEndpoint, getRequestHeaders } from '../../src/api';

Expand All @@ -25,12 +25,12 @@ describe('API', () => {
});

test('getRequestHeaders', () => {
expect(getRequestHeaders(new Dsn(dsnPublic), 'a', '1.0')).toMatchObject({
expect(getRequestHeaders(makeDsn(dsnPublic), 'a', '1.0')).toMatchObject({
'Content-Type': 'application/json',
'X-Sentry-Auth': expect.stringMatching(/^Sentry sentry_version=\d, sentry_client=a\/1\.0, sentry_key=abc$/),
});

expect(getRequestHeaders(new Dsn(legacyDsn), 'a', '1.0')).toMatchObject({
expect(getRequestHeaders(makeDsn(legacyDsn), 'a', '1.0')).toMatchObject({
'Content-Type': 'application/json',
'X-Sentry-Auth': expect.stringMatching(
/^Sentry sentry_version=\d, sentry_client=a\/1\.0, sentry_key=abc, sentry_secret=123$/,
Expand Down Expand Up @@ -119,6 +119,12 @@ describe('API', () => {
});

test('getDsn', () => {
expect(new API(dsnPublic).getDsn()).toEqual(new Dsn(dsnPublic));
expect(new API(dsnPublic).getDsn().host).toEqual(makeDsn(dsnPublic).host);
expect(new API(dsnPublic).getDsn().path).toEqual(makeDsn(dsnPublic).path);
expect(new API(dsnPublic).getDsn().pass).toEqual(makeDsn(dsnPublic).pass);
expect(new API(dsnPublic).getDsn().port).toEqual(makeDsn(dsnPublic).port);
expect(new API(dsnPublic).getDsn().protocol).toEqual(makeDsn(dsnPublic).protocol);
expect(new API(dsnPublic).getDsn().projectId).toEqual(makeDsn(dsnPublic).projectId);
expect(new API(dsnPublic).getDsn().publicKey).toEqual(makeDsn(dsnPublic).publicKey);
});
});
4 changes: 2 additions & 2 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Hub, Scope, Session } from '@sentry/hub';
import { Event, Span, Transport } from '@sentry/types';
import { logger, SentryError, SyncPromise } from '@sentry/utils';
import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils';

import * as integrationModule from '../../src/integration';
import { TestBackend } from '../mocks/backend';
Expand Down Expand Up @@ -67,7 +67,7 @@ describe('BaseClient', () => {
test('returns the Dsn', () => {
expect.assertions(1);
const client = new TestClient({ dsn: PUBLIC_DSN });
expect(client.getDsn()!.toString()).toBe(PUBLIC_DSN);
expect(dsnToString(client.getDsn())).toBe(PUBLIC_DSN);
});

test('allows missing Dsn', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/nextjs/test/integration/next-env.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/// <reference types="next" />
/// <reference types="next/types/global" />
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
Expand Down
4 changes: 2 additions & 2 deletions packages/node/src/backend.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BaseBackend } from '@sentry/core';
import { Event, EventHint, SeverityLevel, Transport, TransportOptions } from '@sentry/types';
import { Dsn } from '@sentry/utils';
import { makeDsn } from '@sentry/utils';

import { eventFromException, eventFromMessage } from './eventbuilder';
import { HTTPSTransport, HTTPTransport } from './transports';
Expand Down Expand Up @@ -35,7 +35,7 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
return super._setupTransport();
}

const dsn = new Dsn(this._options.dsn);
const dsn = makeDsn(this._options.dsn);

const transportOptions: TransportOptions = {
...this._options.transportOptions,
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Dsn } from './dsn';
import { DsnComponents } from './dsn';
import { Event, EventHint } from './event';
import { Integration, IntegrationClass } from './integration';
import { Options } from './options';
Expand Down Expand Up @@ -55,7 +55,7 @@ export interface Client<O extends Options = Options> {
captureSession?(session: Session): void;

/** Returns the current Dsn. */
getDsn(): Dsn | undefined;
getDsn(): DsnComponents | undefined;

/** Returns the current options. */
getOptions(): O;
Expand Down
14 changes: 0 additions & 14 deletions packages/types/src/dsn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,3 @@ export interface DsnComponents {

/** Anything that can be parsed into a Dsn. */
export type DsnLike = string | DsnComponents;

/** The Sentry Dsn, identifying a Sentry instance and project. */
export interface Dsn extends DsnComponents {
/**
* Renders the string representation of this Dsn.
*
* By default, this will render the public representation without the password
* component. To get the deprecated private representation, set `withPassword`
* to true.
*
* @param withPassword When set to true, the password will be included.
*/
toString(withPassword: boolean): string;
}
2 changes: 1 addition & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export { Breadcrumb, BreadcrumbHint } from './breadcrumb';
export { Client } from './client';
export { Context, Contexts } from './context';
export { Dsn, DsnComponents, DsnLike, DsnProtocol } from './dsn';
export { DsnComponents, DsnLike, DsnProtocol } from './dsn';
export { DebugImage, DebugImageType, DebugMeta } from './debugMeta';
export { ExtendedError } from './error';
export { Event, EventHint } from './event';
Expand Down
Loading

0 comments on commit c36de75

Please sign in to comment.