Skip to content

Commit 10048e4

Browse files
kamilogorekkamilogorek
committed
[v7] Simplify transports implementation by using abstract _makeRequest method over base sendRequest
1 parent f403df2 commit 10048e4

File tree

6 files changed

+117
-139
lines changed

6 files changed

+117
-139
lines changed

‎packages/transport-base/src/transport.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { Transport, TransportOptions, TransportResponse, TransportRequest, TransportRequestMaker } from '@sentry/types';
1+
import {
2+
Transport,
3+
TransportOptions,
4+
TransportResponse,
5+
TransportRequest,
6+
TransportMakeRequestResponse,
7+
} from '@sentry/types';
28

39
import { Dsn } from './dsn';
410
import { isRateLimited, updateRateLimits, RateLimits, disabledUntil } from './rateLimit';
@@ -10,15 +16,12 @@ export abstract class BaseTransport {
1016
protected readonly _asyncBuffer: AsyncBuffer<TransportResponse>;
1117
protected _rateLimits: RateLimits = {};
1218

13-
public constructor(public options: TransportOptions) {
14-
this._dsn = new Dsn(this.options.dsn);
15-
this._asyncBuffer = new AsyncBuffer(this.options.bufferSize ?? 30);
19+
public constructor(protected readonly _options: TransportOptions) {
20+
this._dsn = new Dsn(this._options.dsn);
21+
this._asyncBuffer = new AsyncBuffer(this._options.bufferSize ?? 30);
1622
}
1723

18-
public sendRequest<T>(
19-
request: TransportRequest<T>,
20-
requestMaker: TransportRequestMaker<T>,
21-
): PromiseLike<TransportResponse> {
24+
public sendRequest<T>(request: TransportRequest<T>): PromiseLike<TransportResponse> {
2225
if (isRateLimited(this._rateLimits, request.type)) {
2326
return Promise.reject(
2427
new Error(
@@ -36,7 +39,7 @@ export abstract class BaseTransport {
3639
* and they are not making any network calls when the buffer is full.
3740
*/
3841
const sendRequestTask = (): PromiseLike<TransportResponse> => {
39-
return requestMaker(request).then(
42+
return this._makeRequest<T>(request).then(
4043
({ body, headers, statusCode, reason }): PromiseLike<TransportResponse> => {
4144
if (headers) {
4245
this._rateLimits = updateRateLimits(this._rateLimits, headers);
@@ -59,6 +62,8 @@ export abstract class BaseTransport {
5962
public flush(timeout: number = 0): PromiseLike<boolean> {
6063
return this._asyncBuffer.drain(timeout);
6164
}
65+
66+
protected abstract _makeRequest<T>(request: TransportRequest<T>): PromiseLike<TransportMakeRequestResponse>;
6267
}
6368

6469
export class NoopTransport implements Transport {

‎packages/transport-fetch/src/index.ts

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,30 @@
11
import { BaseTransport } from '@sentry/transport-base';
2-
import { Transport, TransportOptions, TransportRequest, TransportRequestMaker, TransportResponse } from '@sentry/types';
2+
import { Transport, TransportRequest, TransportMakeRequestResponse } from '@sentry/types';
33

44
export class FetchTransport extends BaseTransport implements Transport {
5-
constructor(private readonly _options: TransportOptions) {
6-
super(_options);
7-
}
8-
9-
public sendRequest<T>(request: TransportRequest<T>): PromiseLike<TransportResponse> {
10-
const requestMaker: TransportRequestMaker<T> = request => {
11-
const body = (request.body as unknown) as BodyInit;
12-
const requestOptions: RequestInit = {
13-
body,
14-
method: 'POST',
15-
referrerPolicy: 'origin',
16-
headers: this._options.headers,
17-
};
18-
19-
if (this._options.credentials) {
20-
requestOptions.credentials = this._options.credentials as RequestCredentials;
21-
}
22-
23-
return fetch(this._dsn.getEnvelopeEndpoint(), requestOptions).then(async response => {
24-
return {
25-
body: await response.text(),
26-
headers: {
27-
'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
28-
'retry-after': response.headers.get('Retry-After'),
29-
},
30-
reason: response.statusText,
31-
statusCode: response.status,
32-
};
33-
});
5+
protected _makeRequest<T>(request: TransportRequest<T>): PromiseLike<TransportMakeRequestResponse> {
6+
const body = (request.body as unknown) as BodyInit;
7+
const requestOptions: RequestInit = {
8+
body,
9+
method: 'POST',
10+
referrerPolicy: 'origin',
11+
headers: this._options.headers,
3412
};
3513

36-
return super.sendRequest(request, requestMaker);
14+
if (this._options.credentials) {
15+
requestOptions.credentials = this._options.credentials as RequestCredentials;
16+
}
17+
18+
return fetch(this._dsn.getEnvelopeEndpoint(), requestOptions).then(async response => {
19+
return {
20+
body: await response.text(),
21+
headers: {
22+
'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
23+
'retry-after': response.headers.get('Retry-After'),
24+
},
25+
reason: response.statusText,
26+
statusCode: response.status,
27+
};
28+
});
3729
}
3830
}

‎packages/transport-http/src/index.ts

Lines changed: 50 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -4,79 +4,71 @@ import { readFileSync } from 'fs';
44
import { URL } from 'url';
55

66
import { BaseTransport } from '@sentry/transport-base';
7-
import { Transport, TransportOptions, TransportRequest, TransportRequestMaker, TransportResponse } from '@sentry/types';
7+
import { Transport, TransportRequest, TransportMakeRequestResponse } from '@sentry/types';
88

99
// TODO: `x-sentry-error` header?
1010
export class HTTPTransport extends BaseTransport implements Transport {
1111
private _certs?: Buffer;
1212

13-
constructor(private readonly _options: TransportOptions) {
14-
super(_options);
15-
}
16-
17-
public sendRequest<T>(request: TransportRequest<T>): PromiseLike<TransportResponse> {
18-
const requestMaker: TransportRequestMaker<T> = request => {
19-
return new Promise((resolve, reject) => {
20-
const { hostname, pathname, port, protocol } = new URL(this._dsn.getEnvelopeEndpoint());
21-
const httpModule = protocol === 'https:' ? https : http;
22-
const proxy = this._options.proxy || process.env.http_proxy;
23-
const agent = proxy
24-
? new (require('https-proxy-agent'))(proxy)
25-
: new httpModule.Agent({ keepAlive: false, maxSockets: 30, timeout: 2000 });
13+
protected _makeRequest<T>(request: TransportRequest<T>): PromiseLike<TransportMakeRequestResponse> {
14+
return new Promise((resolve, reject) => {
15+
const { hostname, pathname, port, protocol } = new URL(this._dsn.getEnvelopeEndpoint());
16+
const httpModule = protocol === 'https:' ? https : http;
17+
const proxy = this._options.proxy || process.env.http_proxy;
18+
const agent = proxy
19+
? new (require('https-proxy-agent'))(proxy)
20+
: new httpModule.Agent({ keepAlive: false, maxSockets: 30, timeout: 2000 });
2621

27-
const requestOptions: https.RequestOptions = {
28-
agent,
29-
hostname,
30-
method: 'POST',
31-
path: pathname,
32-
port,
33-
protocol,
34-
headers: {
35-
...this._options.headers,
36-
...this._dsn.getEnvelopeEndpointAuthHeaders(),
37-
},
38-
};
22+
const requestOptions: https.RequestOptions = {
23+
agent,
24+
hostname,
25+
method: 'POST',
26+
path: pathname,
27+
port,
28+
protocol,
29+
headers: {
30+
...this._options.headers,
31+
...this._dsn.getEnvelopeEndpointAuthHeaders(),
32+
},
33+
};
3934

40-
if (this._options.caCerts) {
41-
if (!this._certs) {
42-
try {
43-
this._certs = readFileSync(this._options.caCerts);
44-
} catch (_oO) {
45-
// no-empty
46-
}
35+
if (this._options.caCerts) {
36+
if (!this._certs) {
37+
try {
38+
this._certs = readFileSync(this._options.caCerts);
39+
} catch (_oO) {
40+
// no-empty
4741
}
42+
}
4843

49-
if (this._certs) {
50-
requestOptions.ca = this._certs;
51-
}
44+
if (this._certs) {
45+
requestOptions.ca = this._certs;
5246
}
47+
}
5348

54-
// TODO: Add gzip compresison
55-
const req = httpModule.request(requestOptions, (res: http.IncomingMessage) => {
56-
res.setEncoding('utf8');
49+
// TODO: Add gzip compresison
50+
const req = httpModule.request(requestOptions, (res: http.IncomingMessage) => {
51+
res.setEncoding('utf8');
5752

58-
let body = '';
59-
res.on('data', chunk => {
60-
body += chunk;
61-
});
62-
res.on('end', () => {
63-
resolve({
64-
body,
65-
headers: {
66-
'x-sentry-rate-limits': res.headers['X-Sentry-Rate-Limits'] as string,
67-
'retry-after': res.headers['Retry-After'] as string,
68-
},
69-
reason: res.statusMessage,
70-
statusCode: res.statusCode || 500,
71-
});
53+
let body = '';
54+
res.on('data', chunk => {
55+
body += chunk;
56+
});
57+
res.on('end', () => {
58+
resolve({
59+
body,
60+
headers: {
61+
'x-sentry-rate-limits': res.headers['X-Sentry-Rate-Limits'] as string,
62+
'retry-after': res.headers['Retry-After'] as string,
63+
},
64+
reason: res.statusMessage,
65+
statusCode: res.statusCode || 500,
7266
});
7367
});
74-
75-
req.on('error', error => reject(error));
76-
req.end(request.body);
7768
});
78-
};
7969

80-
return super.sendRequest(request, requestMaker);
70+
req.on('error', error => reject(error));
71+
req.end(request.body);
72+
});
8173
}
8274
}

‎packages/transport-xhr/src/index.ts

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,36 @@
11
import { BaseTransport } from '@sentry/transport-base';
2-
import { Transport, TransportOptions, TransportRequest, TransportRequestMaker, TransportResponse } from '@sentry/types';
2+
import { Transport, TransportRequest, TransportMakeRequestResponse } from '@sentry/types';
33

44
export class XHRTransport extends BaseTransport implements Transport {
5-
constructor(private readonly _options: TransportOptions) {
6-
super(_options);
7-
}
8-
9-
public sendRequest<T>(request: TransportRequest<T>): PromiseLike<TransportResponse> {
10-
const requestMaker: TransportRequestMaker<T> = request => {
11-
return new Promise((resolve, reject) => {
12-
const xhr = new XMLHttpRequest();
5+
protected _makeRequest<T>(request: TransportRequest<T>): PromiseLike<TransportMakeRequestResponse> {
6+
return new Promise((resolve, reject) => {
7+
const xhr = new XMLHttpRequest();
138

14-
xhr.onerror = error => reject(error);
15-
xhr.onreadystatechange = (): void => {
16-
if (xhr.readyState === 4) {
17-
resolve({
18-
body: xhr.responseText,
19-
headers: {
20-
'x-sentry-rate-limits': xhr.getResponseHeader('X-Sentry-Rate-Limits'),
21-
'retry-after': xhr.getResponseHeader('Retry-After'),
22-
},
23-
reason: xhr.statusText,
24-
statusCode: xhr.status,
25-
});
26-
}
27-
};
28-
29-
xhr.open('POST', this._dsn.getEnvelopeEndpoint());
30-
// "When using setRequestHeader(), you must call it after calling open(), but before calling send()."
31-
// https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
32-
for (const header in this._options.headers) {
33-
if (Object.prototype.hasOwnProperty.call(this._options.headers, header)) {
34-
xhr.setRequestHeader(header, this._options.headers[header]);
35-
}
9+
xhr.onerror = error => reject(error);
10+
xhr.onreadystatechange = (): void => {
11+
if (xhr.readyState === 4) {
12+
resolve({
13+
body: xhr.responseText,
14+
headers: {
15+
'x-sentry-rate-limits': xhr.getResponseHeader('X-Sentry-Rate-Limits'),
16+
'retry-after': xhr.getResponseHeader('Retry-After'),
17+
},
18+
reason: xhr.statusText,
19+
statusCode: xhr.status,
20+
});
3621
}
37-
const body = (request.body as unknown) as BodyInit;
38-
xhr.send(body);
39-
});
40-
};
22+
};
4123

42-
return super.sendRequest(request, requestMaker);
24+
xhr.open('POST', this._dsn.getEnvelopeEndpoint());
25+
// "When using setRequestHeader(), you must call it after calling open(), but before calling send()."
26+
// https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
27+
for (const header in this._options.headers) {
28+
if (Object.prototype.hasOwnProperty.call(this._options.headers, header)) {
29+
xhr.setRequestHeader(header, this._options.headers[header]);
30+
}
31+
}
32+
const body = (request.body as unknown) as BodyInit;
33+
xhr.send(body);
34+
});
4335
}
4436
}

‎packages/types/src/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ export {
4141
Transport,
4242
TransportOptions,
4343
TransportRequest,
44-
TransportRequestMaker,
4544
TransportResponse,
46-
TransportMakerResponse,
45+
TransportMakeRequestResponse,
4746
} from './transport';
4847
export { User } from './user';
4948
export { WrappedFunction } from './wrappedfunction';

‎packages/types/src/transport.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,13 @@ export type TransportRequest<T> = {
1414
type: EventType;
1515
};
1616

17-
export type TransportMakerResponse = {
17+
export type TransportMakeRequestResponse = {
1818
body?: string;
1919
headers?: Record<string, string | null>;
2020
reason?: string;
2121
statusCode: number;
2222
};
2323

24-
export type TransportRequestMaker<T> = (request: TransportRequest<T>) => PromiseLike<TransportMakerResponse>;
25-
2624
export type TransportResponse = {
2725
status: ResponseStatus;
2826
reason?: string;

0 commit comments

Comments
 (0)