feat: Honor no_proxy env variable

packages/node/src/transports/base.ts

@@ -1,5 +1,5 @@
 import { API, eventToSentryRequest, SDK_VERSION } from '@sentry/core';
-import { Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
+import { DsnProtocol, Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
 import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sentry/utils';
 import * as fs from 'fs';
 import * as http from 'http';
@@ -70,6 +70,34 @@ export abstract class BaseTransport implements Transport {
     return this._buffer.drain(timeout);
   }
 
+  /**
+   * Extracts proxy settings from client options and env variables.
+   *
+   * Honors `no_proxy` env variable with the highest priority to allow for hosts exclusion.
+   *
+   * An order of priority for available protocols is:
+   * `http`  => `options.httpProxy` | `process.env.http_proxy`
+   * `https` => `options.httpsProxy` | `options.httpProxy` | `process.env.https_proxy` | `process.env.http_proxy`
+   */
+  protected _getProxy(protocol: DsnProtocol): string | undefined {
+    const { no_proxy, http_proxy, https_proxy } = process.env;
+    const { httpProxy, httpsProxy } = this.options;
+    const proxy = protocol === 'http' ? httpProxy || http_proxy : httpsProxy || httpProxy || https_proxy || http_proxy;
+
+    if (!no_proxy) {
+      return proxy;
+    }
+
+    const { host, port } = this._api.getDsn();
+    for (const np of no_proxy.split(',')) {
+      if (host.endsWith(np) || `${host}:${port}`.endsWith(np)) {
+        return;
+      }
+    }
+
+    return proxy;
+  }
+
   /** Returns a build request option object used by request */
   protected _getRequestOptions(uri: url.URL): http.RequestOptions | https.RequestOptions {
     const headers = {

packages/node/src/transports/http.ts

@@ -9,7 +9,7 @@ export class HTTPTransport extends BaseTransport {
   /** Create a new instance and set this.agent */
   public constructor(public options: TransportOptions) {
     super(options);
-    const proxy = options.httpProxy || process.env.http_proxy;
+    const proxy = this._getProxy('http');
     this.module = http;
     this.client = proxy
       ? (new (require('https-proxy-agent'))(proxy) as http.Agent)

packages/node/src/transports/https.ts

@@ -9,7 +9,7 @@ export class HTTPSTransport extends BaseTransport {
   /** Create a new instance and set this.agent */
   public constructor(public options: TransportOptions) {
     super(options);
-    const proxy = options.httpsProxy || options.httpProxy || process.env.https_proxy || process.env.http_proxy;
+    const proxy = this._getProxy('https');
     this.module = https;
     this.client = proxy
       ? (new (require('https-proxy-agent'))(proxy) as https.Agent)

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

@@ -1,5 +1,6 @@
 import { TransportOptions } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
+import * as http from 'http';
 import * as HttpsProxyAgent from 'https-proxy-agent';
 
 import { HTTPTransport } from '../../src/transports/http';
@@ -147,22 +148,71 @@ describe('HTTPTransport', () => {
     expect(requestOptions.headers).toEqual(expect.objectContaining({ a: 'b' }));
   });
 
-  test('http proxy', async () => {
-    mockReturnCode = 200;
-    const transport = createTransport({
-      dsn,
-      httpProxy: 'http://example.com:8080',
+  describe('proxy', () => {
+    test('can be configured through client option', async () => {
+      const transport = createTransport({
+        dsn,
+        httpProxy: 'http://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(false);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
     });
-    await transport.sendEvent({
-      message: 'test',
+
+    test('can be configured through env variables option', async () => {
+      process.env.http_proxy = 'http://example.com:8080';
+      const transport = createTransport({
+        dsn,
+        httpProxy: 'http://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(false);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
+      delete process.env.http_proxy;
     });
 
-    const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
-    assertBasicOptions(requestOptions);
-    expect(requestOptions.agent).toBeInstanceOf(HttpsProxyAgent);
-    expect(requestOptions.agent.secureProxy).toEqual(false);
-    expect(requestOptions.agent.proxy).toEqual(
-      expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }),
-    );
+    test('client options have priority over env variables', async () => {
+      process.env.http_proxy = 'http://env-example.com:8080';
+      const transport = createTransport({
+        dsn,
+        httpProxy: 'http://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(false);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'http:', port: 8080, host: 'example.com' }));
+      delete process.env.http_proxy;
+    });
+
+    test('no_proxy allows for skipping specific hosts', async () => {
+      process.env.no_proxy = 'sentry.io';
+      const transport = createTransport({
+        dsn,
+        httpProxy: 'http://example.com:8080',
+      });
+      expect(transport.client).toBeInstanceOf(http.Agent);
+    });
+
+    test('no_proxy works with a port', async () => {
+      process.env.http_proxy = 'http://example.com:8080';
+      process.env.no_proxy = 'sentry.io:8989';
+      const transport = createTransport({
+        dsn,
+      });
+      expect(transport.client).toBeInstanceOf(http.Agent);
+      delete process.env.http_proxy;
+    });
+
+    test('no_proxy works with multiple comma-separated hosts', async () => {
+      process.env.http_proxy = 'http://example.com:8080';
+      process.env.no_proxy = 'example.com,sentry.io,wat.com:1337';
+      const transport = createTransport({
+        dsn,
+      });
+      expect(transport.client).toBeInstanceOf(http.Agent);
+      delete process.env.http_proxy;
+    });
   });
 });

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

@@ -1,5 +1,6 @@
 import { TransportOptions } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
+import * as https from 'https';
 import * as HttpsProxyAgent from 'https-proxy-agent';
 
 import { HTTPSTransport } from '../../src/transports/https';
@@ -153,36 +154,110 @@ describe('HTTPSTransport', () => {
     expect(requestOptions.headers).toEqual(expect.objectContaining({ a: 'b' }));
   });
 
-  test('https proxy', async () => {
-    mockReturnCode = 200;
-    const transport = createTransport({
-      dsn,
-      httpsProxy: 'https://example.com:8080',
+  describe('proxy', () => {
+    test('can be configured through client option', async () => {
+      const transport = createTransport({
+        dsn,
+        httpsProxy: 'https://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(true);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
     });
-    await transport.sendEvent({
-      message: 'test',
+
+    test('can be configured through env variables option', async () => {
+      process.env.https_proxy = 'https://example.com:8080';
+      const transport = createTransport({
+        dsn,
+        httpsProxy: 'https://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(true);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
+      delete process.env.https_proxy;
     });
 
-    const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
-    assertBasicOptions(requestOptions);
-    expect(requestOptions.agent).toBeInstanceOf(HttpsProxyAgent);
-    expect(requestOptions.agent.secureProxy).toEqual(true);
-    expect(requestOptions.agent.proxy).toEqual(
-      expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }),
-    );
-  });
+    test('https proxies have priority in client option', async () => {
+      const transport = createTransport({
+        dsn,
+        httpProxy: 'http://unsecure-example.com:8080',
+        httpsProxy: 'https://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(true);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
+    });
 
-  test('tls certificate', async () => {
-    mockReturnCode = 200;
-    const transport = createTransport({
-      caCerts: './some/path.pem',
-      dsn,
+    test('https proxies have priority in env variables', async () => {
+      process.env.http_proxy = 'http://unsecure-example.com:8080';
+      process.env.https_proxy = 'https://example.com:8080';
+      const transport = createTransport({
+        dsn,
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(true);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
+      delete process.env.http_proxy;
+      delete process.env.https_proxy;
     });
-    await transport.sendEvent({
-      message: 'test',
+
+    test('client options have priority over env variables', async () => {
+      process.env.https_proxy = 'https://env-example.com:8080';
+      const transport = createTransport({
+        dsn,
+        httpsProxy: 'https://example.com:8080',
+      });
+      const client = (transport.client as unknown) as { proxy: Record<string, string | number>; secureProxy: boolean };
+      expect(client).toBeInstanceOf(HttpsProxyAgent);
+      expect(client.secureProxy).toEqual(true);
+      expect(client.proxy).toEqual(expect.objectContaining({ protocol: 'https:', port: 8080, host: 'example.com' }));
+      delete process.env.https_proxy;
+    });
+
+    test('no_proxy allows for skipping specific hosts', async () => {
+      process.env.no_proxy = 'sentry.io';
+      const transport = createTransport({
+        dsn,
+        httpsProxy: 'https://example.com:8080',
+      });
+      expect(transport.client).toBeInstanceOf(https.Agent);
+    });
+
+    test('no_proxy works with a port', async () => {
+      process.env.https_proxy = 'https://example.com:8080';
+      process.env.no_proxy = 'sentry.io:8989';
+      const transport = createTransport({
+        dsn,
+      });
+      expect(transport.client).toBeInstanceOf(https.Agent);
+      delete process.env.https_proxy;
+    });
+
+    test('no_proxy works with multiple comma-separated hosts', async () => {
+      process.env.http_proxy = 'https://example.com:8080';
+      process.env.no_proxy = 'example.com,sentry.io,wat.com:1337';
+      const transport = createTransport({
+        dsn,
+      });
+      expect(transport.client).toBeInstanceOf(https.Agent);
+      delete process.env.https_proxy;
+    });
+
+    test('can configure tls certificate through client option', async () => {
+      mockReturnCode = 200;
+      const transport = createTransport({
+        caCerts: './some/path.pem',
+        dsn,
+      });
+      await transport.sendEvent({
+        message: 'test',
+      });
+      const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
+      expect(requestOptions.ca).toEqual('mockedCert');
     });
-    const requestOptions = (transport.module!.request as jest.Mock).mock.calls[0][0];
-    assertBasicOptions(requestOptions);
-    expect(requestOptions.ca).toEqual('mockedCert');
   });
 });