Add protocol detection for get/request calls without explicit protocol

packages/node/src/integrations/http.ts

@@ -94,7 +94,7 @@ function _createWrappedRequestMethodFactory(
       // eslint-disable-next-line @typescript-eslint/no-this-alias
       const httpModule = this;
 
-      const requestArgs = normalizeRequestArgs(args);
+      const requestArgs = normalizeRequestArgs(this, args);
       const requestOptions = requestArgs[0];
       const requestUrl = extractUrl(requestOptions);
 

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

@@ -1,5 +1,6 @@
 import { getCurrentHub } from '@sentry/core';
 import * as http from 'http';
+import * as https from 'https';
 import { URL } from 'url';
 
 /**
@@ -122,6 +123,7 @@ export function urlToOptions(url: URL): RequestOptions {
  * @returns Equivalent args of the form [ RequestOptions ] or [ RequestOptions, RequestCallback ].
  */
 export function normalizeRequestArgs(
+  httpModule: typeof http | typeof https,
   requestArgs: RequestMethodArgs,
 ): [RequestOptions] | [RequestOptions, RequestCallback] {
   let callback, requestOptions;
@@ -145,6 +147,17 @@ export function normalizeRequestArgs(
     requestOptions = { ...requestOptions, ...requestArgs[1] };
   }
 
+  // Figure out the protocol if it's currently missing
+  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;
+    /* eslint-enable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any */
+  }
+
   // return args in standardized form
   if (callback) {
     return [requestOptions, callback];

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

@@ -2,12 +2,17 @@ import * as sentryCore from '@sentry/core';
 import { Hub } from '@sentry/hub';
 import * as hubModule from '@sentry/hub';
 import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing';
+import { parseSemver } from '@sentry/utils';
 import * as http from 'http';
+import * as https from 'https';
 import * as nock from 'nock';
 
+import { Breadcrumb } from '../../src';
 import { NodeClient } from '../../src/client';
 import { Http as HttpIntegration } from '../../src/integrations/http';
 
+const NODE_VERSION = parseSemver(process.versions.node);
+
 describe('tracing', () => {
   function createTransactionOnScope() {
     const hub = new Hub(
@@ -91,3 +96,83 @@ describe('tracing', () => {
     expect(sentryTraceHeader).not.toBeDefined();
   });
 });
+
+describe('default protocols', () => {
+  function captureBreadcrumb(key: string): Promise<Breadcrumb> {
+    const hub = new Hub();
+    jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
+
+    let resolve: (value: Breadcrumb | PromiseLike<Breadcrumb>) => void;
+    const p = new Promise<Breadcrumb>(r => {
+      resolve = r;
+    });
+    hub.bindClient(
+      new NodeClient({
+        dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012',
+        integrations: [new HttpIntegration({ breadcrumbs: true })],
+        beforeBreadcrumb: (b: Breadcrumb) => {
+          if ((b.data?.url as string).includes(key)) {
+            resolve(b);
+          }
+          return b;
+        },
+      }),
+    );
+
+    return p;
+  }
+
+  it('http module', async () => {
+    const key = 'catrunners';
+    const p = captureBreadcrumb(key);
+
+    nock(`http://${key}.ingest.sentry.io`)
+      .get('/api/123122332/store/')
+      .reply(200);
+
+    http.get({
+      host: `${key}.ingest.sentry.io`,
+      path: '/api/123122332/store/',
+    });
+
+    const b = await p;
+    expect(b.data?.url).toEqual(expect.stringContaining('http://'));
+  });
+
+  it('https module', async () => {
+    const key = 'catcatchers';
+    const p = captureBreadcrumb(key);
+
+    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
+    // regardless (if it asked to mock a https:// url), preventing the
+    // request from reaching the integrations patch of the `http` module.
+    // The result is test failures in Node v8 and lower.
+    //
+    // We can work around this by telling giving nock a http:// url, so it
+    // only patches the `http` module, then Nodes usage of the `http` module
+    // in the `https` module results in both nock's and the integrations
+    // patch being called. All this relies on nock not properly checking
+    // the agent passed to `http.get` / `http.request`, thus resulting in it
+    // intercepting a https:// request with http:// mock. It's a safe bet
+    // 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:';
+    }
+    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,
+    });
+
+    const b = await p;
+    expect(b.data?.url).toEqual(expect.stringContaining('https://'));
+  });
+});