Skip to content

Commit b61dcdf

Browse files
1999kamilogorek
andauthored
feat: Allow for overriding custom UrlParser in Node.js transports (#3612)
* Add URL container support * Replace URLContainer with URLParser * Make URLParser required with default value to avoid breaking changes to existing transports * Update test suite name Co-authored-by: Kamil Ogórek <[email protected]>
1 parent e46451e commit b61dcdf

File tree

3 files changed

+47
-6
lines changed

3 files changed

+47
-6
lines changed

packages/node/src/transports/base.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sent
1515
import * as fs from 'fs';
1616
import * as http from 'http';
1717
import * as https from 'https';
18-
import * as url from 'url';
18+
import { URL } from 'url';
1919

2020
import { SDK_NAME } from '../version';
2121

@@ -30,7 +30,7 @@ export interface HTTPModule {
3030
* @param callback Callback when request is finished
3131
*/
3232
request(
33-
options: http.RequestOptions | https.RequestOptions | string | url.URL,
33+
options: http.RequestOptions | https.RequestOptions | string | URL,
3434
callback?: (res: http.IncomingMessage) => void,
3535
): http.ClientRequest;
3636

@@ -39,12 +39,15 @@ export interface HTTPModule {
3939
// versions:
4040

4141
// request(
42-
// url: string | url.URL,
42+
// url: string | URL,
4343
// options: http.RequestOptions | https.RequestOptions,
4444
// callback?: (res: http.IncomingMessage) => void,
4545
// ): http.ClientRequest;
4646
}
4747

48+
export type URLParts = Pick<URL, 'hostname' | 'pathname' | 'port' | 'protocol'>;
49+
export type UrlParser = (url: string) => URLParts;
50+
4851
const CATEGORY_MAPPING: {
4952
[key in SentryRequestType]: string;
5053
} = {
@@ -76,6 +79,9 @@ export abstract class BaseTransport implements Transport {
7679
this._api = new API(options.dsn, options._metadata);
7780
}
7881

82+
/** Default function used to parse URLs */
83+
public urlParser: UrlParser = url => new URL(url);
84+
7985
/**
8086
* @inheritDoc
8187
*/
@@ -119,12 +125,12 @@ export abstract class BaseTransport implements Transport {
119125
}
120126

121127
/** Returns a build request option object used by request */
122-
protected _getRequestOptions(uri: url.URL): http.RequestOptions | https.RequestOptions {
128+
protected _getRequestOptions(urlParts: URLParts): http.RequestOptions | https.RequestOptions {
123129
const headers = {
124130
...this._api.getRequestHeaders(SDK_NAME, SDK_VERSION),
125131
...this.options.headers,
126132
};
127-
const { hostname, pathname, port, protocol } = uri;
133+
const { hostname, pathname, port, protocol } = urlParts;
128134
// See https://github.com/nodejs/node/blob/38146e717fed2fabe3aacb6540d839475e0ce1c6/lib/internal/url.js#L1268-L1290
129135
// We ignore the query string on purpose
130136
const path = `${pathname}`;
@@ -224,7 +230,7 @@ export abstract class BaseTransport implements Transport {
224230
if (!this.module) {
225231
throw new SentryError('No module available');
226232
}
227-
const options = this._getRequestOptions(new url.URL(sentryReq.url));
233+
const options = this._getRequestOptions(this.urlParser(sentryReq.url));
228234
const req = this.module.request(options, (res: http.IncomingMessage) => {
229235
const statusCode = res.statusCode || 500;
230236
const status = Status.fromHttpCode(statusCode);
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { CustomUrlTransport } from './transports';
2+
3+
describe('Custom transport', () => {
4+
describe('URL parser support', () => {
5+
const noop = () => null;
6+
const sampleDsn = 'https://[email protected]/path/1';
7+
8+
test('use URL parser for sendEvent() method', async () => {
9+
const urlParser = jest.fn();
10+
const transport = new CustomUrlTransport({ dsn: sampleDsn }, urlParser);
11+
await transport.sendEvent({}).catch(noop);
12+
13+
expect(urlParser).toHaveBeenCalled();
14+
});
15+
16+
test('use URL parser for sendSession() method', async () => {
17+
const urlParser = jest.fn();
18+
const transport = new CustomUrlTransport({ dsn: sampleDsn }, urlParser);
19+
await transport.sendSession({ aggregates: [] }).then(noop, noop);
20+
21+
expect(urlParser).toHaveBeenCalled();
22+
});
23+
});
24+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { TransportOptions } from '@sentry/types';
2+
3+
import { HTTPTransport } from '../../../src/transports';
4+
import { UrlParser } from '../../../src/transports/base';
5+
6+
export class CustomUrlTransport extends HTTPTransport {
7+
public constructor(public options: TransportOptions, urlParser: UrlParser) {
8+
super(options);
9+
this.urlParser = urlParser;
10+
}
11+
}

0 commit comments

Comments
 (0)