Skip to content
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

Add URL container support to Node.JS transports #3612

Merged
merged 10 commits into from
Jun 7, 2021
18 changes: 12 additions & 6 deletions packages/node/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sent
import * as fs from 'fs';
import * as http from 'http';
import * as https from 'https';
import * as url from 'url';
import { URL } from 'url';

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

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

Expand All @@ -39,12 +39,15 @@ export interface HTTPModule {
// versions:

// request(
// url: string | url.URL,
// url: string | URL,
// options: http.RequestOptions | https.RequestOptions,
// callback?: (res: http.IncomingMessage) => void,
// ): http.ClientRequest;
}

export type URLParts = Pick<URL, 'hostname' | 'pathname' | 'port' | 'protocol'>;
export type UrlParser = (url: string) => URLParts;

const CATEGORY_MAPPING: {
[key in SentryRequestType]: string;
} = {
Expand Down Expand Up @@ -76,6 +79,9 @@ export abstract class BaseTransport implements Transport {
this._api = new API(options.dsn, options._metadata);
}

/** Default function used to parse URLs */
public urlParser: UrlParser = url => new URL(url);

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -119,12 +125,12 @@ export abstract class BaseTransport implements Transport {
}

/** Returns a build request option object used by request */
protected _getRequestOptions(uri: url.URL): http.RequestOptions | https.RequestOptions {
protected _getRequestOptions(urlParts: URLParts): http.RequestOptions | https.RequestOptions {
const headers = {
...this._api.getRequestHeaders(SDK_NAME, SDK_VERSION),
...this.options.headers,
};
const { hostname, pathname, port, protocol } = uri;
const { hostname, pathname, port, protocol } = urlParts;
// See https://github.com/nodejs/node/blob/38146e717fed2fabe3aacb6540d839475e0ce1c6/lib/internal/url.js#L1268-L1290
// We ignore the query string on purpose
const path = `${pathname}`;
Expand Down Expand Up @@ -224,7 +230,7 @@ export abstract class BaseTransport implements Transport {
if (!this.module) {
throw new SentryError('No module available');
}
const options = this._getRequestOptions(new url.URL(sentryReq.url));
const options = this._getRequestOptions(this.urlParser(sentryReq.url));
const req = this.module.request(options, (res: http.IncomingMessage) => {
const statusCode = res.statusCode || 500;
const status = Status.fromHttpCode(statusCode);
Expand Down
24 changes: 24 additions & 0 deletions packages/node/test/transports/custom/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { CustomUrlTransport } from './transports';

describe('Custom transport', () => {
describe('URL parser support', () => {
const noop = () => null;
const sampleDsn = 'https://username@sentry.tld/path/1';

test('use URL parser for sendEvent() method', async () => {
const urlParser = jest.fn();
const transport = new CustomUrlTransport({ dsn: sampleDsn }, urlParser);
await transport.sendEvent({}).catch(noop);

expect(urlParser).toHaveBeenCalled();
});

test('use URL parser for sendSession() method', async () => {
const urlParser = jest.fn();
const transport = new CustomUrlTransport({ dsn: sampleDsn }, urlParser);
await transport.sendSession({ aggregates: [] }).then(noop, noop);

expect(urlParser).toHaveBeenCalled();
});
});
});
11 changes: 11 additions & 0 deletions packages/node/test/transports/custom/transports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { TransportOptions } from '@sentry/types';

import { HTTPTransport } from '../../../src/transports';
import { UrlParser } from '../../../src/transports/base';

export class CustomUrlTransport extends HTTPTransport {
public constructor(public options: TransportOptions, urlParser: UrlParser) {
super(options);
this.urlParser = urlParser;
}
}