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

feat: HTTP(S) Proxy support for Node #1761

Merged
merged 3 commits into from
Nov 23, 2018
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- [node] HTTP(S) Proxy support

## 4.3.4

- [utils] fix: Broken tslib import - Fixes #1757
Expand Down
18 changes: 9 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,31 @@
"packages/utils"
],
"devDependencies": {
"@google-cloud/storage": "^1.7.0",
"@google-cloud/storage": "^2.3.1",
"@types/chai": "^4.1.3",
"@types/jest": "^22.2.3",
"@types/jest": "^23.3.9",
"@types/mocha": "^5.2.0",
"@types/node": "^10.0.2",
"@types/raven": "^2.5.1",
"@types/sinon": "^4.3.1",
"@types/sinon": "^5.0.7",
"chai": "^4.1.2",
"codecov": "^3.0.2",
"danger": "^4.0.2",
"danger": "^6.1.5",
"danger-plugin-tslint": "^2.0.0",
"jest": "^22.4.3",
"jest": "^23.6.0",
"karma-sinon": "^1.0.5",
"lerna": "3.4.0",
"lerna": "3.4.3",
"mocha": "^4.1.0",
"npm-run-all": "^4.1.2",
"prettier": "^1.14.0",
"prettier-check": "^2.0.0",
"replace-in-file": "^3.4.0",
"rimraf": "^2.6.2",
"sinon": "^5.0.3",
"ts-jest": "^22.4.4",
"sinon": "^7.1.1",
"ts-jest": "^23.10.5",
"tslint": "^5.11.0",
"tslint-language-service": "^0.9.9",
"typedoc": "^0.12.0",
"typedoc": "^0.13.0",
"typedoc-plugin-monorepo": "^0.1.0",
"typescript": "^3.0.1"
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
],
"globals": {
"ts-jest": {
"tsConfigFile": "./tsconfig.json"
"tsConfig": "./tsconfig.json",
"diagnostics": false
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion packages/hub/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@
],
"globals": {
"ts-jest": {
"tsConfigFile": "./tsconfig.json"
"tsConfig": "./tsconfig.json",
"diagnostics": false
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion packages/minimal/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@
],
"globals": {
"ts-jest": {
"tsConfigFile": "./tsconfig.json"
"tsConfig": "./tsconfig.json",
"diagnostics": false
}
}
},
Expand Down
4 changes: 3 additions & 1 deletion packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@sentry/utils": "4.3.4",
"@types/stack-trace": "0.0.29",
"cookie": "0.3.1",
"https-proxy-agent": "^2.2.1",
"lsmod": "1.0.0",
"stack-trace": "0.0.10",
"tslib": "^1.9.3"
Expand Down Expand Up @@ -69,7 +70,8 @@
],
"globals": {
"ts-jest": {
"tsConfigFile": "./tsconfig.json"
"tsConfig": "./tsconfig.json",
"diagnostics": false
}
}
},
Expand Down
19 changes: 19 additions & 0 deletions packages/node/src/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { HTTPSTransport, HTTPTransport } from './transports';
* @see NodeClient for more information.
*/
export interface NodeOptions extends Options {
[key: string]: any;

/** Callback that is executed when a fatal global error occurs. */
onFatalError?(error: Error): void;

Expand All @@ -19,6 +21,15 @@ export interface NodeOptions extends Options {

/** Maximum time to wait to drain the request queue, before the process is allowed to exit. */
shutdownTimeout?: number;

/** Set a HTTP proxy that should be used for outbound requests. */
httpProxy?: string;

/** Set a HTTPS proxy that should be used for outbound requests. */
httpsProxy?: string;

/** HTTPS proxy certificates path */
caCerts?: string;
}

/** The Sentry Node SDK Backend. */
Expand Down Expand Up @@ -101,6 +112,14 @@ export class NodeBackend extends BaseBackend<NodeOptions> {

if (!this.transport) {
const transportOptions = this.options.transportOptions ? this.options.transportOptions : { dsn };
const clientOptions = ['httpProxy', 'httpsProxy', 'caCerts'];

for (const option of clientOptions) {
if (this.options[option]) {
transportOptions[option] = transportOptions[option] || this.options[option];
}
}

this.transport = this.options.transport
? new this.options.transport({ dsn })
: dsn.protocol === 'http'
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/declarations.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
declare module 'lsmod';
declare module 'https-proxy-agent';
18 changes: 15 additions & 3 deletions packages/node/src/transports/base.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { API, SentryError } from '@sentry/core';
import { SentryEvent, SentryResponse, Status, Transport, TransportOptions } from '@sentry/types';
import { serialize } from '@sentry/utils/object';
import * as fs from 'fs';
import * as http from 'http';
import * as https from 'https';
import * as url from 'url';
Expand All @@ -20,7 +21,10 @@ export abstract class BaseTransport implements Transport {
protected api: API;

/** The Agent used for corresponding transport */
protected client: http.Agent | https.Agent | undefined;
public module?: HTTPRequest;

/** The Agent used for corresponding transport */
public client?: http.Agent | https.Agent;

/** Create instance and set this.dsn */
public constructor(public options: TransportOptions) {
Expand All @@ -33,9 +37,11 @@ export abstract class BaseTransport implements Transport {
...this.api.getRequestHeaders(SDK_NAME, SDK_VERSION),
...this.options.headers,
};

const dsn = this.api.getDsn();
return {

const options: {
[key: string]: any;
} = {
agent: this.client,
headers,
hostname: dsn.host,
Expand All @@ -44,6 +50,12 @@ export abstract class BaseTransport implements Transport {
port: dsn.port,
protocol: `${dsn.protocol}:`,
};

if (this.options.caCerts) {
options.ca = fs.readFileSync(this.options.caCerts);
}

return options;
}

/** JSDoc */
Expand Down
16 changes: 13 additions & 3 deletions packages/node/src/transports/http.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
import { SentryError } from '@sentry/core';
import { SentryEvent, SentryResponse, TransportOptions } from '@sentry/types';
import * as http from 'http';
import * as HttpsProxyAgent from 'https-proxy-agent';
import { BaseTransport } from './base';

/** /** Node http module transport */
/** Node http module transport */
export class HTTPTransport extends BaseTransport {
/** Create a new instance and set this.agent */
public constructor(public options: TransportOptions) {
super(options);
this.client = new http.Agent({ keepAlive: true, maxSockets: 100 });
this.module = http;
const proxy = options.httpProxy || process.env.http_proxy;
this.client = proxy
? // tslint:disable-next-line:no-unsafe-any
(new HttpsProxyAgent(proxy) as http.Agent)
: new http.Agent({ keepAlive: true, maxSockets: 100 });
}

/**
* @inheritDoc
*/
public async captureEvent(event: SentryEvent): Promise<SentryResponse> {
return this.sendWithModule(http, event);
if (!this.module) {
throw new SentryError('No module available in HTTPTransport');
}
return this.sendWithModule(this.module, event);
}
}
15 changes: 13 additions & 2 deletions packages/node/src/transports/https.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
import { SentryError } from '@sentry/core';
import { SentryEvent, SentryResponse, TransportOptions } from '@sentry/types';
import * as https from 'https';
import * as HttpsProxyAgent from 'https-proxy-agent';
import { BaseTransport } from './base';

/** Node https module transport */
export class HTTPSTransport extends BaseTransport {
/** Create a new instance and set this.agent */
public constructor(public options: TransportOptions) {
super(options);
this.client = new https.Agent({ keepAlive: true, maxSockets: 100 });
this.module = https;
const proxy = options.httpsProxy || options.httpProxy || process.env.https_proxy || process.env.http_proxy;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the order making sense here.

Copy link
Contributor Author

@kamilogorek kamilogorek Nov 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From our docs:

When set a proxy can be configured that should be used for outbound requests. This is also used for HTTPS requests unless a separate https-proxy is configured.

I think that user specified options should always override env variables, thus order is like this.

this.client = proxy
? // tslint:disable-next-line:no-unsafe-any
(new HttpsProxyAgent(proxy) as https.Agent)
: new https.Agent({ keepAlive: true, maxSockets: 100 });
}

/**
* @inheritDoc
*/
public async captureEvent(event: SentryEvent): Promise<SentryResponse> {
return this.sendWithModule(https, event);
if (!this.module) {
throw new SentryError('No module available in HTTPSTransport');
}

return this.sendWithModule(this.module, event);
}
}
Loading