Skip to content

Commit

Permalink
fix(node): Revert to dynamic require call to fix monkey patching (#…
Browse files Browse the repository at this point in the history
…7430)

Partially revert #7377 which caused monkey patching errors when patching the native `http` and `https` modules in the Node SDK (#7425). Similarly, also our Serverless SDK was subjected to the same problem (#7421). 

The problem is that `import` doesn't permit monkey patching of the imported (`http(s)`) module, producing this error:

```bash
TypeError: Cannot assign to read only property 'get' of object '[object Module]'
```

I tried using a dynamic import instead but got the same result. So it seems like we can only use `require` here :(
  • Loading branch information
Lms24 authored Mar 13, 2023
1 parent 9c2971b commit fe1231a
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {
parseSemver,
stringMatchesSomePattern,
} from '@sentry/utils';
import * as http from 'http';
import * as https from 'https';
import type * as http from 'http';
import type * as https from 'https';
import { LRUMap } from 'lru_map';

import type { NodeClient } from '../client';
Expand Down Expand Up @@ -101,17 +101,25 @@ export class Http implements Integration {
// and we will no longer have to do this optional merge, we can just pass `this._tracing` directly.
const tracingOptions = this._tracing ? { ...clientOptions, ...this._tracing } : undefined;

const wrappedHttpHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, tracingOptions, http);
fill(http, 'get', wrappedHttpHandlerMaker);
fill(http, 'request', wrappedHttpHandlerMaker);
// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpModule = require('http');
const wrappedHttpHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, tracingOptions, httpModule);
fill(httpModule, 'get', wrappedHttpHandlerMaker);
fill(httpModule, 'request', wrappedHttpHandlerMaker);

// NOTE: Prior to Node 9, `https` used internals of `http` module, thus we don't patch it.
// If we do, we'd get double breadcrumbs and double spans for `https` calls.
// It has been changed in Node 9, so for all versions equal and above, we patch `https` separately.
if (NODE_VERSION.major && NODE_VERSION.major > 8) {
const wrappedHttpsHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, tracingOptions, https);
fill(https, 'get', wrappedHttpsHandlerMaker);
fill(https, 'request', wrappedHttpsHandlerMaker);
// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpsModule = require('https');
const wrappedHttpsHandlerMaker = _createWrappedRequestMethodFactory(
this._breadcrumbs,
tracingOptions,
httpsModule,
);
fill(httpsModule, 'get', wrappedHttpsHandlerMaker);
fill(httpsModule, 'request', wrappedHttpsHandlerMaker);
}
}
}
Expand Down

0 comments on commit fe1231a

Please sign in to comment.