Skip to content

Commit ac20799

Browse files
authored
fix(node): Prioritize globalAgent while figuring out protocol. (#4087)
* fix(node): Prioritize `globalAgent` while figuring out protocol.
1 parent 0ae48f0 commit ac20799

File tree

2 files changed

+50
-6
lines changed

2 files changed

+50
-6
lines changed

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import { getCurrentHub } from '@sentry/core';
2+
import { parseSemver } from '@sentry/utils';
23
import * as http from 'http';
34
import * as https from 'https';
45
import { URL } from 'url';
56

7+
const NODE_VERSION = parseSemver(process.versions.node);
8+
69
/**
710
* Checks whether given url points to Sentry server
811
* @param url url to verify
@@ -151,10 +154,23 @@ export function normalizeRequestArgs(
151154
if (requestOptions.protocol === undefined) {
152155
// Worst case we end up populating protocol with undefined, which it already is
153156
/* eslint-disable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any */
154-
requestOptions.protocol =
155-
(requestOptions.agent as any)?.protocol ||
156-
(requestOptions._defaultAgent as any)?.protocol ||
157-
(httpModule?.globalAgent as any)?.protocol;
157+
158+
// NOTE: Prior to Node 9, `https` used internals of `http` module, thus we don't patch it.
159+
// Because of that, we cannot rely on `httpModule` to provide us with valid protocol,
160+
// as it will always return `http`, even when using `https` module.
161+
//
162+
// See test/integrations/http.test.ts for more details on Node <=v8 protocol issue.
163+
if (NODE_VERSION.major && NODE_VERSION.major > 8) {
164+
requestOptions.protocol =
165+
(httpModule?.globalAgent as any)?.protocol ||
166+
(requestOptions.agent as any)?.protocol ||
167+
(requestOptions._defaultAgent as any)?.protocol;
168+
} else {
169+
requestOptions.protocol =
170+
(requestOptions.agent as any)?.protocol ||
171+
(requestOptions._defaultAgent as any)?.protocol ||
172+
(httpModule?.globalAgent as any)?.protocol;
173+
}
158174
/* eslint-enable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any */
159175
}
160176

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sen
55
import { parseSemver } from '@sentry/utils';
66
import * as http from 'http';
77
import * as https from 'https';
8+
import * as HttpsProxyAgent from 'https-proxy-agent';
89
import * as nock from 'nock';
910

1011
import { Breadcrumb } from '../../src';
@@ -143,7 +144,7 @@ describe('default protocols', () => {
143144
const key = 'catcatchers';
144145
const p = captureBreadcrumb(key);
145146

146-
let nockProtocol = 'https:';
147+
let nockProtocol = 'https';
147148
// NOTE: Prior to Node 9, `https` used internals of `http` module, so
148149
// the integration doesn't patch the `https` module. However this then
149150
// causes issues with nock, because nock will patch the `https` module
@@ -160,7 +161,7 @@ describe('default protocols', () => {
160161
// because the latest versions of nock no longer support Node v8 and lower,
161162
// so won't bother dealing with this old Node edge case.
162163
if (NODE_VERSION.major && NODE_VERSION.major < 9) {
163-
nockProtocol = 'http:';
164+
nockProtocol = 'http';
164165
}
165166
nock(`${nockProtocol}://${key}.ingest.sentry.io`)
166167
.get('/api/123122332/store/')
@@ -175,4 +176,31 @@ describe('default protocols', () => {
175176
const b = await p;
176177
expect(b.data?.url).toEqual(expect.stringContaining('https://'));
177178
});
179+
180+
it('makes https request over http proxy', async () => {
181+
const key = 'catcatchers';
182+
const p = captureBreadcrumb(key);
183+
let nockProtocol = 'https';
184+
185+
const proxy = 'http://<PROXY_URL>:3128';
186+
const agent = new HttpsProxyAgent(proxy);
187+
188+
if (NODE_VERSION.major && NODE_VERSION.major < 9) {
189+
nockProtocol = 'http';
190+
}
191+
192+
nock(`${nockProtocol}://${key}.ingest.sentry.io`)
193+
.get('/api/123122332/store/')
194+
.reply(200);
195+
196+
https.get({
197+
host: `${key}.ingest.sentry.io`,
198+
path: '/api/123122332/store/',
199+
timeout: 300,
200+
agent,
201+
});
202+
203+
const b = await p;
204+
expect(b.data?.url).toEqual(expect.stringContaining('https://'));
205+
});
178206
});

0 commit comments

Comments
 (0)