fix: Make rewritten http integration work in pre v9 node versions

kamilogorek · kamilogorek · commit 74eacc002252 · 2019-11-01T11:28:28.000+01:00
diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -1,9 +1,11 @@
 import { getCurrentHub, Span } from '@sentry/core';
 import { Integration } from '@sentry/types';
-import { fill } from '@sentry/utils';
+import { fill, parseSemver } from '@sentry/utils';
 import * as http from 'http';
 import * as https from 'https';
 
+const NODE_VERSION = parseSemver(process.versions.node);
+
 /** http module integration */
 export class Http implements Integration {
   /**
@@ -48,9 +50,14 @@ export class Http implements Integration {
     fill(httpModule, 'get', handlerWrapper);
     fill(httpModule, 'request', handlerWrapper);
 
-    const httpsModule = require('https');
-    fill(httpsModule, 'get', handlerWrapper);
-    fill(httpsModule, 'request', handlerWrapper);
+    // 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 httpsModule = require('https');
+      fill(httpsModule, 'get', handlerWrapper);
+      fill(httpsModule, 'request', handlerWrapper);
+    }
   }
 }
 
@@ -74,7 +81,7 @@ function createHandlerWrapper(
       let span: Span;
       if (tracingEnabled) {
         span = getCurrentHub().startSpan({
-          description: `${typeof options === 'string' ? 'GET' : options.method}|${requestUrl}`,
+          description: `${typeof options === 'string' || !options.method ? 'GET' : options.method}|${requestUrl}`,
           op: 'request',
         });
       }
diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts
@@ -342,3 +342,35 @@ function _htmlElementAsString(elem: HTMLElement): string {
 export function timestampWithMs(): number {
   return new Date().getTime() / 1000;
 }
+
+// https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
+const SEMVER_REGEXP = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/;
+
+/**
+ * Represents Semantic Versioning object
+ */
+interface SemVer {
+  major?: number;
+  minor?: number;
+  patch?: number;
+  prerelease?: string;
+  buildmetadata?: string;
+}
+
+/**
+ * Parses input into a SemVer interface
+ * @param input string representation of a semver version
+ */
+export function parseSemver(input: string): SemVer {
+  const match = input.match(SEMVER_REGEXP) || [];
+  const major = parseInt(match[1], 10);
+  const minor = parseInt(match[2], 10);
+  const patch = parseInt(match[3], 10);
+  return {
+    buildmetadata: match[5],
+    major: isNaN(major) ? undefined : major,
+    minor: isNaN(minor) ? undefined : minor,
+    patch: isNaN(patch) ? undefined : patch,
+    prerelease: match[4],
+  };
+}
