Skip to content

feat(transports): trim down some code in the transports #4303

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
Dec 16, 2021
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
16 changes: 6 additions & 10 deletions packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,10 @@ import {

import { sendReport } from './utils';

const CATEGORY_MAPPING: {
[key in SentryRequestType]: string;
} = {
event: 'error',
transaction: 'transaction',
session: 'session',
attachment: 'attachment',
};
function requestTypeToCategory(ty: SentryRequestType): string {
const tyStr = ty as string;
return tyStr === 'event' ? 'error' : tyStr;
}

const global = getGlobalObject<Window>();

Expand Down Expand Up @@ -93,7 +89,7 @@ export abstract class BaseTransport implements Transport {
// We could use nested maps, but it's much easier to read and type this way.
// 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 = `${CATEGORY_MAPPING[category]}:${reason}`;
const key = `${requestTypeToCategory(category)}:${reason}`;
logger.log(`Adding outcome: ${key}`);
this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
}
Expand Down Expand Up @@ -180,7 +176,7 @@ export abstract class BaseTransport implements Transport {
* Gets the time that given category is disabled until for rate limiting
*/
protected _disabledUntil(requestType: SentryRequestType): Date {
const category = CATEGORY_MAPPING[requestType];
const category = requestTypeToCategory(requestType);
return this._rateLimits[category] || this._rateLimits.all;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/hub/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ export class Session implements SessionInterface {
errors: this.errors,
did: typeof this.did === 'number' || typeof this.did === 'string' ? `${this.did}` : undefined,
duration: this.duration,
attrs: dropUndefinedKeys({
attrs: {
release: this.release,
environment: this.environment,
ip_address: this.ipAddress,
user_agent: this.userAgent,
}),
},
});
}
}
30 changes: 17 additions & 13 deletions packages/utils/src/dsn.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DsnComponents, DsnLike, DsnProtocol } from '@sentry/types';

import { isDebugBuild } from './env';
import { SentryError } from './error';

/** Regular expression used to parse a Dsn. */
Expand Down Expand Up @@ -102,22 +103,25 @@ export class Dsn implements DsnComponents {

/** Validates this Dsn and throws on error. */
private _validate(): void {
['protocol', 'publicKey', 'host', 'projectId'].forEach(component => {
if (!this[component as keyof DsnComponents]) {
throw new SentryError(`${ERROR_MESSAGE}: ${component} missing`);
// we only validate in debug mode. This will fail later anyways.
if (isDebugBuild()) {
['protocol', 'publicKey', 'host', 'projectId'].forEach(component => {
if (!this[component as keyof DsnComponents]) {
throw new SentryError(`${ERROR_MESSAGE}: ${component} missing`);
}
});

if (!this.projectId.match(/^\d+$/)) {
throw new SentryError(`${ERROR_MESSAGE}: Invalid projectId ${this.projectId}`);
}
});

if (!this.projectId.match(/^\d+$/)) {
throw new SentryError(`${ERROR_MESSAGE}: Invalid projectId ${this.projectId}`);
}

if (this.protocol !== 'http' && this.protocol !== 'https') {
throw new SentryError(`${ERROR_MESSAGE}: Invalid protocol ${this.protocol}`);
}
if (this.protocol !== 'http' && this.protocol !== 'https') {
throw new SentryError(`${ERROR_MESSAGE}: Invalid protocol ${this.protocol}`);
}

if (this.port && isNaN(parseInt(this.port, 10))) {
throw new SentryError(`${ERROR_MESSAGE}: Invalid port ${this.port}`);
if (this.port && isNaN(parseInt(this.port, 10))) {
throw new SentryError(`${ERROR_MESSAGE}: Invalid port ${this.port}`);
}
}
}
}
16 changes: 11 additions & 5 deletions packages/utils/test/dsn.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { isDebugBuild } from '@sentry/utils';

import { Dsn } from '../src/dsn';
import { SentryError } from '../src/error';

function testIf(condition: boolean): jest.It {
return condition ? test : test.skip;
}

describe('Dsn', () => {
describe('fromComponents', () => {
test('applies all components', () => {
Expand Down Expand Up @@ -37,7 +43,7 @@ describe('Dsn', () => {
expect(dsn.projectId).toBe('123');
});

test('throws for missing components', () => {
testIf(isDebugBuild())('throws for missing components', () => {
expect(
() =>
new Dsn({
Expand Down Expand Up @@ -76,7 +82,7 @@ describe('Dsn', () => {
).toThrow(SentryError);
});

test('throws for invalid components', () => {
testIf(isDebugBuild())('throws for invalid components', () => {
expect(
() =>
new Dsn({
Expand Down Expand Up @@ -144,18 +150,18 @@ describe('Dsn', () => {
expect(dsn.projectId).toBe('321');
});

test('throws when provided invalid Dsn', () => {
testIf(isDebugBuild())('throws when provided invalid Dsn', () => {
expect(() => new Dsn('[email protected]')).toThrow(SentryError);
});

test('throws without mandatory fields', () => {
testIf(isDebugBuild())('throws without mandatory fields', () => {
expect(() => new Dsn('://[email protected]/123')).toThrow(SentryError);
expect(() => new Dsn('https://@sentry.io/123')).toThrow(SentryError);
expect(() => new Dsn('https://abc@123')).toThrow(SentryError);
expect(() => new Dsn('https://[email protected]/')).toThrow(SentryError);
});

test('throws for invalid fields', () => {
testIf(isDebugBuild())('throws for invalid fields', () => {
expect(() => new Dsn('httpx://[email protected]/123')).toThrow(SentryError);
expect(() => new Dsn('httpx://[email protected]:xxx/123')).toThrow(SentryError);
expect(() => new Dsn('http://[email protected]/abc')).toThrow(SentryError);
Expand Down