diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 77e72228ee66..ad80462f3c25 100644 --- a/packages/node/src/integrations/http.ts +++ b/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); diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index 26da20ae86e3..e5d73ebc1d5a 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/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]; diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 38370650daac..ad15c5f62450 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/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 { + const hub = new Hub(); + jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + + let resolve: (value: Breadcrumb | PromiseLike) => void; + const p = new Promise(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://')); + }); +});