diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index 15f2c01d6bf2..bb0e9a76cf00 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/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 */ } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index ad15c5f62450..824768481504 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/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://: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://')); + }); });