Merge branch 'master' into tag-lcp-elements

Author: dashed

packages/browser/src/sdk.ts

@@ -189,23 +189,25 @@ function startSessionTracking(): void {
 
   const hub = getCurrentHub();
 
-  if ('startSession' in hub) {
-    // The only way for this to be false is for there to be a version mismatch between @sentry/browser (>= 6.0.0) and
-    // @sentry/hub (< 5.27.0). In the simple case, there won't ever be such a mismatch, because the two packages are
-    // pinned at the same version in package.json, but there are edge cases where it's possible'. See
-    // https://github.com/getsentry/sentry-javascript/issues/3234 and
-    // https://github.com/getsentry/sentry-javascript/issues/3207.
-
-    hub.startSession();
-    hub.captureSession();
-
-    // We want to create a session for every navigation as well
-    addInstrumentationHandler({
-      callback: () => {
-        hub.startSession();
-        hub.captureSession();
-      },
-      type: 'history',
-    });
+  // The only way for this to be false is for there to be a version mismatch between @sentry/browser (>= 6.0.0) and
+  // @sentry/hub (< 5.27.0). In the simple case, there won't ever be such a mismatch, because the two packages are
+  // pinned at the same version in package.json, but there are edge cases where it's possible. See
+  // https://github.com/getsentry/sentry-javascript/issues/3207 and
+  // https://github.com/getsentry/sentry-javascript/issues/3234 and
+  // https://github.com/getsentry/sentry-javascript/issues/3278.
+  if (typeof hub.startSession !== 'function' || typeof hub.captureSession !== 'function') {
+    return;
   }
+
+  hub.startSession();
+  hub.captureSession();
+
+  // We want to create a session for every navigation as well
+  addInstrumentationHandler({
+    callback: () => {
+      hub.startSession();
+      hub.captureSession();
+    },
+    type: 'history',
+  });
 }

packages/node/src/sdk.ts

@@ -149,7 +149,7 @@ export async function close(timeout?: number): Promise<boolean> {
 }
 
 /**
- * A function that returns a Sentry release string dynamically from env variables
+ * Returns a release dynamically from environment variables.
  */
 export function getSentryRelease(): string | undefined {
   // Always read first as Sentry takes this as precedence

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/manual/apm-transaction/main.js

@@ -105,8 +105,9 @@ app.get('/trace', async (_req, res, next) => {
 app.use(Tracing.finish());
 app.use(Sentry.Handlers.errorHandler());
 
-const server = app.listen(1231, () => {
-  http.get('http://localhost:1231/trace', res => {
+const server = app.listen(0, () => {
+  const port = server.address().port;
+  http.get(`http://localhost:${port}/trace`, res => {
     server.close();
   });
 });

packages/node/test/manual/express-scope-separation/start.js

@@ -18,6 +18,7 @@ class DummyTransport {
 
     if (!remaining) {
       console.error('SUCCESS: All scopes contain correct tags');
+      server.close();
       process.exit(0);
     }
 
@@ -83,8 +84,9 @@ app.get('/baz', req => {
 
 app.use(Sentry.Handlers.errorHandler());
 
-app.listen(1121);
-
-http.get('http://localhost:1121/foo');
-http.get('http://localhost:1121/bar');
-http.get('http://localhost:1121/baz');
+const server = app.listen(0, () => {
+  const port = server.address().port;
+  http.get(`http://localhost:${port}/foo`);
+  http.get(`http://localhost:${port}/bar`);
+  http.get(`http://localhost:${port}/baz`);
+});

packages/node/test/manual/memory-leak/express-patient.js

@@ -146,7 +146,8 @@ app.use((err, req, res, next) => {
   return res.status(500).send('oh no there was an error: ' + err.message);
 });
 
-const server = app.listen(5140, () => {
-  process._rawDebug('patient is waiting to be poked on port 5140');
+const server = app.listen(0, () => {
+  const port = server.address().port;
+  process._rawDebug(`patient is waiting to be poked on port ${port}`);
   memwatch.gc();
 });

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');
   });
 });