Http Integration + HttpsProxyAgent with http: url + calling https.get with global this = TypeError

[wereHamster]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using? If you use the CDN bundles, please specify the exact bundle (e.g. bundle.tracing.min.js) in your SDK setup.

@sentry/node

SDK Version

7.36.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

Sentry.init({
  integrations: [new Sentry.Integrations.Http({ tracing: true })],
  tracesSampleRate: 1.0,
})

Steps to Reproduce

const Sentry = require('@sentry/node')
require('@sentry/tracing')
const HttpsProxyAgent = require('https-proxy-agent')

Sentry.init({
  integrations: [new Sentry.Integrations.Http({ tracing: true })],
  tracesSampleRate: 1.0,
})

const get = require('https').get

get({
  agent: new HttpsProxyAgent.HttpsProxyAgent('http://localhost:8080'),
  hostname: 'apple.com',
})

Expected Result

no crash

Actual Result

Crash

TypeError [ERR_INVALID_PROTOCOL]: Protocol "http:" not supported. Expected "https:"
    at new NodeError (node:internal/errors:393:5)
    at new ClientRequest (node:_http_client:186:11)
    at request (node:https:360:10)
    at get (node:https:394:15)
    at wrappedMethod (…/node_modules/@sentry/node/cjs/integrations/http.js:208:10)
    at Object.<anonymous> (…/test.cjs:12:1)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)

The issue that the code that's hooking into https.get expects this to be the https module but due to how the function is called this is the globalThis.

With this being the https module, the code in normalizeRequestArgs() will initialize protocol correctly (this.globalAgent.protocol). But with this being global scope globalThis.globalAgent does not exist, so the function uses requestOptions.agent.protocol which in my code is http:.

Workaround

Setting the protocol of the agent explicitly to https: seems to make it work.

const agent = new HttpsProxyAgent.HttpsProxyAgent('http://localhost:8080')
agent.protocol = 'https:'

[lforst]

Hi, I have a suspicion that the real issue here is that you're detaching the this context from .get() by doing const get = require('https').get and calling get without binding this back to the method. If the SDK won't crash from this, something else is bound to down the line. Some linters will even scream at you for doing that for exactly the reason you're running into right now. I recommend you do something like this instead:

const https = require('https');

https.get({
  agent: new HttpsProxyAgent.HttpsProxyAgent('http://localhost:8080/'),
  hostname: 'apple.com',
})

If you really need to redefine get for some reason, do this:

const https = require('https');

const get = https.get.bind(https);

get({
  agent: new HttpsProxyAgent.HttpsProxyAgent('http://localhost:8080/'),
  hostname: 'apple.com',
})

[wereHamster]

Question is, is it allowed in Node.js or not? Also, it's not me but it's the @slack/bolt package that's doing it internally.

[wereHamster]

Oh, not even that. It's the ws library (which I believe is quite widely used): https://github.com/websockets/ws/blob/2862c2fef1064940c1deb9c4aa961cb76c279feb/lib/websocket.js#LL707C3-L707C59

[lforst]

@wereHamster The more I think about it the more I think you're right. Pushed up a fix #7135 which should make this more reliable.

[wereHamster]

That's exactly the fix I was thinking about (passing this explicitly to the wrapper-maker)… will test once released.