Skip to content

Commit c91b32c

Browse files
kamilogorekkamilogorek
authored
feat: Honor no_proxy env variable (#3412)
* feat: Honor no_proxy env variable * Better docs
1 parent 21917f9 commit c91b32c

File tree

5 files changed

+195
-42
lines changed

5 files changed

+195
-42
lines changed

‎packages/node/src/transports/base.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { API, eventToSentryRequest, SDK_VERSION } from '@sentry/core';
2-
import { Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
2+
import { DsnProtocol, Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
33
import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sentry/utils';
44
import * as fs from 'fs';
55
import * as http from 'http';
@@ -70,6 +70,34 @@ export abstract class BaseTransport implements Transport {
7070
return this._buffer.drain(timeout);
7171
}
7272

73+
/**
74+
* Extracts proxy settings from client options and env variables.
75+
*
76+
* Honors `no_proxy` env variable with the highest priority to allow for hosts exclusion.
77+
*
78+
* An order of priority for available protocols is:
79+
* `http` => `options.httpProxy` | `process.env.http_proxy`
80+
* `https` => `options.httpsProxy` | `options.httpProxy` | `process.env.https_proxy` | `process.env.http_proxy`
81+
*/
82+
protected _getProxy(protocol: DsnProtocol): string | undefined {
83+
const { no_proxy, http_proxy, https_proxy } = process.env;
84+
const { httpProxy, httpsProxy } = this.options;
85+
const proxy = protocol === 'http' ? httpProxy || http_proxy : httpsProxy || httpProxy || https_proxy || http_proxy;
86+
87+
if (!no_proxy) {
88+
return proxy;
89+
}
90+
91+
const { host, port } = this._api.getDsn();
92+
for (const np of no_proxy.split(',')) {
93+
if (host.endsWith(np) || `${host}:${port}`.endsWith(np)) {
94+
return;
95+
}
96+
}
97+
98+
return proxy;
99+
}
100+
73101
/** Returns a build request option object used by request */
74102
protected _getRequestOptions(uri: url.URL): http.RequestOptions | https.RequestOptions {
75103
const headers = {

‎packages/node/src/transports/http.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class HTTPTransport extends BaseTransport {
99
/** Create a new instance and set this.agent */
1010
public constructor(public options: TransportOptions) {
1111
super(options);
12-
const proxy = options.httpProxy || process.env.http_proxy;
12+
const proxy = this._getProxy('http');
1313
this.module = http;
1414
this.client = proxy
1515
? (new (require('https-proxy-agent'))(proxy) as http.Agent)

‎packages/node/src/transports/https.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class HTTPSTransport extends BaseTransport {
99
/** Create a new instance and set this.agent */
1010
public constructor(public options: TransportOptions) {
1111
super(options);
12-
const proxy = options.httpsProxy || options.httpProxy || process.env.https_proxy || process.env.http_proxy;
12+
const proxy = this._getProxy('https');
1313
this.module = https;
1414
this.client = proxy
1515
? (new (require('https-proxy-agent'))(proxy) as https.Agent)

‎packages/node/test/transports/http.test.ts

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { TransportOptions } from '@sentry/types';
22
import { SentryError } from '@sentry/utils';
3+
import * as http from 'http';
34
import * as HttpsProxyAgent from 'https-proxy-agent';
45

56
import { HTTPTransport } from '../../src/transports/http';
@@ -147,22 +148,71 @@ describe('HTTPTransport', () => {
147148
expect(requestOptions.headers).toEqual(expect.objectContaining({ a: 'b' }));
148149
});
149150

150-
test('http proxy', async () => {
151-
mockReturnCode = 200;
152-
const transport = createTransport({
153-
dsn,
154-
httpProxy: 'http://example.com:8080',
151+
describe('proxy', () => {
152+
test('can be configured through client option', async () => {
153+
const transport = createTransport({
154+
dsn,
155+
httpProxy: 'http://example.com:8080',
156+
});
157+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
158+
expect(client).toBeInstanceOf(HttpsProxyAgent);
159+
expect(client.secureProxy).toEqual(false);
160+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
155161
});
156-
await transport.sendEvent({
157-
message: 'test',
162+
163+
test('can be configured through env variables option', async () => {
164+
process.env.http_proxy = 'http://example.com:8080';
165+
const transport = createTransport({
166+
dsn,
167+
httpProxy: 'http://example.com:8080',
168+
});
169+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
170+
expect(client).toBeInstanceOf(HttpsProxyAgent);
171+
expect(client.secureProxy).toEqual(false);
172+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
173+
delete process.env.http_proxy;
158174
});
159175

160-
const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
161-
assertBasicOptions(requestOptions);
162-
expect(requestOptions.agent).toBeInstanceOf(HttpsProxyAgent);
163-
expect(requestOptions.agent.secureProxy).toEqual(false);
164-
expect(requestOptions.agent.proxy).toEqual(
165-
expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }),
166-
);
176+
test('client options have priority over env variables', async () => {
177+
process.env.http_proxy = 'http://env-example.com:8080';
178+
const transport = createTransport({
179+
dsn,
180+
httpProxy: 'http://example.com:8080',
181+
});
182+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
183+
expect(client).toBeInstanceOf(HttpsProxyAgent);
184+
expect(client.secureProxy).toEqual(false);
185+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
186+
delete process.env.http_proxy;
187+
});
188+
189+
test('no_proxy allows for skipping specific hosts', async () => {
190+
process.env.no_proxy = 'sentry.io';
191+
const transport = createTransport({
192+
dsn,
193+
httpProxy: 'http://example.com:8080',
194+
});
195+
expect(transport.client).toBeInstanceOf(http.Agent);
196+
});
197+
198+
test('no_proxy works with a port', async () => {
199+
process.env.http_proxy = 'http://example.com:8080';
200+
process.env.no_proxy = 'sentry.io:8989';
201+
const transport = createTransport({
202+
dsn,
203+
});
204+
expect(transport.client).toBeInstanceOf(http.Agent);
205+
delete process.env.http_proxy;
206+
});
207+
208+
test('no_proxy works with multiple comma-separated hosts', async () => {
209+
process.env.http_proxy = 'http://example.com:8080';
210+
process.env.no_proxy = 'example.com,sentry.io,wat.com:1337';
211+
const transport = createTransport({
212+
dsn,
213+
});
214+
expect(transport.client).toBeInstanceOf(http.Agent);
215+
delete process.env.http_proxy;
216+
});
167217
});
168218
});

‎packages/node/test/transports/https.test.ts

Lines changed: 100 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { TransportOptions } from '@sentry/types';
22
import { SentryError } from '@sentry/utils';
3+
import * as https from 'https';
34
import * as HttpsProxyAgent from 'https-proxy-agent';
45

56
import { HTTPSTransport } from '../../src/transports/https';
@@ -153,36 +154,110 @@ describe('HTTPSTransport', () => {
153154
expect(requestOptions.headers).toEqual(expect.objectContaining({ a: 'b' }));
154155
});
155156

156-
test('https proxy', async () => {
157-
mockReturnCode = 200;
158-
const transport = createTransport({
159-
dsn,
160-
httpsProxy: 'https://example.com:8080',
157+
describe('proxy', () => {
158+
test('can be configured through client option', async () => {
159+
const transport = createTransport({
160+
dsn,
161+
httpsProxy: 'https://example.com:8080',
162+
});
163+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
164+
expect(client).toBeInstanceOf(HttpsProxyAgent);
165+
expect(client.secureProxy).toEqual(true);
166+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
161167
});
162-
await transport.sendEvent({
163-
message: 'test',
168+
169+
test('can be configured through env variables option', async () => {
170+
process.env.https_proxy = 'https://example.com:8080';
171+
const transport = createTransport({
172+
dsn,
173+
httpsProxy: 'https://example.com:8080',
174+
});
175+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
176+
expect(client).toBeInstanceOf(HttpsProxyAgent);
177+
expect(client.secureProxy).toEqual(true);
178+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
179+
delete process.env.https_proxy;
164180
});
165181

166-
const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
167-
assertBasicOptions(requestOptions);
168-
expect(requestOptions.agent).toBeInstanceOf(HttpsProxyAgent);
169-
expect(requestOptions.agent.secureProxy).toEqual(true);
170-
expect(requestOptions.agent.proxy).toEqual(
171-
expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }),
172-
);
173-
});
182+
test('https proxies have priority in client option', async () => {
183+
const transport = createTransport({
184+
dsn,
185+
httpProxy: 'http://unsecure-example.com:8080',
186+
httpsProxy: 'https://example.com:8080',
187+
});
188+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
189+
expect(client).toBeInstanceOf(HttpsProxyAgent);
190+
expect(client.secureProxy).toEqual(true);
191+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
192+
});
174193

175-
test('tls certificate', async () => {
176-
mockReturnCode = 200;
177-
const transport = createTransport({
178-
caCerts: './some/path.pem',
179-
dsn,
194+
test('https proxies have priority in env variables', async () => {
195+
process.env.http_proxy = 'http://unsecure-example.com:8080';
196+
process.env.https_proxy = 'https://example.com:8080';
197+
const transport = createTransport({
198+
dsn,
199+
});
200+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
201+
expect(client).toBeInstanceOf(HttpsProxyAgent);
202+
expect(client.secureProxy).toEqual(true);
203+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
204+
delete process.env.http_proxy;
205+
delete process.env.https_proxy;
180206
});
181-
await transport.sendEvent({
182-
message: 'test',
207+
208+
test('client options have priority over env variables', async () => {
209+
process.env.https_proxy = 'https://env-example.com:8080';
210+
const transport = createTransport({
211+
dsn,
212+
httpsProxy: 'https://example.com:8080',
213+
});
214+
const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
215+
expect(client).toBeInstanceOf(HttpsProxyAgent);
216+
expect(client.secureProxy).toEqual(true);
217+
expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
218+
delete process.env.https_proxy;
219+
});
220+
221+
test('no_proxy allows for skipping specific hosts', async () => {
222+
process.env.no_proxy = 'sentry.io';
223+
const transport = createTransport({
224+
dsn,
225+
httpsProxy: 'https://example.com:8080',
226+
});
227+
expect(transport.client).toBeInstanceOf(https.Agent);
228+
});
229+
230+
test('no_proxy works with a port', async () => {
231+
process.env.https_proxy = 'https://example.com:8080';
232+
process.env.no_proxy = 'sentry.io:8989';
233+
const transport = createTransport({
234+
dsn,
235+
});
236+
expect(transport.client).toBeInstanceOf(https.Agent);
237+
delete process.env.https_proxy;
238+
});
239+
240+
test('no_proxy works with multiple comma-separated hosts', async () => {
241+
process.env.http_proxy = 'https://example.com:8080';
242+
process.env.no_proxy = 'example.com,sentry.io,wat.com:1337';
243+
const transport = createTransport({
244+
dsn,
245+
});
246+
expect(transport.client).toBeInstanceOf(https.Agent);
247+
delete process.env.https_proxy;
248+
});
249+
250+
test('can configure tls certificate through client option', async () => {
251+
mockReturnCode = 200;
252+
const transport = createTransport({
253+
caCerts: './some/path.pem',
254+
dsn,
255+
});
256+
await transport.sendEvent({
257+
message: 'test',
258+
});
259+
const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
260+
expect(requestOptions.ca).toEqual('mockedCert');
183261
});
184-
const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
185-
assertBasicOptions(requestOptions);
186-
expect(requestOptions.ca).toEqual('mockedCert');
187262
});
188263
});

0 commit comments

Comments
 (0)