fix(node): Prioritize globalAgent while figuring out protocol.

packages/node/src/integrations/utils/http.ts

@@ -1,8 +1,11 @@
 import { getCurrentHub } from '@sentry/core';
+import { parseSemver } from '@sentry/utils';
 import * as http from 'http';
 import * as https from 'https';
 import { URL } from 'url';
 
+const NODE_VERSION = parseSemver(process.versions.node);
+
 /**
  * Checks whether given url points to Sentry server
  * @param url url to verify
@@ -151,10 +154,23 @@ export function normalizeRequestArgs(
   if (requestOptions.protocol === undefined) {
     // Worst case we end up populating protocol with undefined, which it already is
     /* eslint-disable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any */
-    requestOptions.protocol =
-      (requestOptions.agent as any)?.protocol ||
-      (requestOptions._defaultAgent as any)?.protocol ||
-      (httpModule?.globalAgent as any)?.protocol;
+
+    // NOTE: Prior to Node 9, `https` used internals of `http` module, thus we don't patch it.
+    // Because of that, we cannot rely on `httpModule` to provide us with valid protocol,
+    // as it will always return `http`, even when using `https` module.
+    //
+    // See test/integrations/http.test.ts for more details on Node <=v8 protocol issue.
+    if (NODE_VERSION.major && NODE_VERSION.major > 8) {
+      requestOptions.protocol =
+        (httpModule?.globalAgent as any)?.protocol ||
+        (requestOptions.agent as any)?.protocol ||
+        (requestOptions._defaultAgent as any)?.protocol;
+    } else {
+      requestOptions.protocol =
+        (requestOptions.agent as any)?.protocol ||
+        (requestOptions._defaultAgent as any)?.protocol ||
+        (httpModule?.globalAgent as any)?.protocol;
+    }
     /* eslint-enable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any */
   }
 

packages/node/test/integrations/http.test.ts

@@ -5,6 +5,7 @@ import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sen
 import { parseSemver } from '@sentry/utils';
 import * as http from 'http';
 import * as https from 'https';
+import * as HttpsProxyAgent from 'https-proxy-agent';
 import * as nock from 'nock';
 
 import { Breadcrumb } from '../../src';
@@ -143,7 +144,7 @@ describe('default protocols', () => {
     const key = 'catcatchers';
     const p = captureBreadcrumb(key);
 
-    let nockProtocol = 'https:';
+    let nockProtocol = 'https';
     // NOTE: Prior to Node 9, `https` used internals of `http` module, so
     // the integration doesn't patch the `https` module. However this then
     // causes issues with nock, because nock will patch the `https` module
@@ -160,7 +161,7 @@ describe('default protocols', () => {
     // because the latest versions of nock no longer support Node v8 and lower,
     // so won't bother dealing with this old Node edge case.
     if (NODE_VERSION.major && NODE_VERSION.major < 9) {
-      nockProtocol = 'http:';
+      nockProtocol = 'http';
     }
     nock(`${nockProtocol}://${key}.ingest.sentry.io`)
       .get('/api/123122332/store/')
@@ -175,4 +176,31 @@ describe('default protocols', () => {
     const b = await p;
     expect(b.data?.url).toEqual(expect.stringContaining('https://'));
   });
+
+  it('makes https request over http proxy', async () => {
+    const key = 'catcatchers';
+    const p = captureBreadcrumb(key);
+    let nockProtocol = 'https';
+
+    const proxy = 'http://<PROXY_URL>:3128';
+    const agent = new HttpsProxyAgent(proxy);
+
+    if (NODE_VERSION.major && NODE_VERSION.major < 9) {
+      nockProtocol = 'http';
+    }
+
+    nock(`${nockProtocol}://${key}.ingest.sentry.io`)
+      .get('/api/123122332/store/')
+      .reply(200);
+
+    https.get({
+      host: `${key}.ingest.sentry.io`,
+      path: '/api/123122332/store/',
+      timeout: 300,
+      agent,
+    });
+
+    const b = await p;
+    expect(b.data?.url).toEqual(expect.stringContaining('https://'));
+  });
 });