Repository: honeycombio/beeline-nodejs
Pull request #87 (2018-08-10 14:59:07)

fix async hooks breakage on early v10
Turns out the code in #82 is necessary for at least early v10 usage (see https://github.com/honeycombio/beeline-nodejs/issues/71#issuecomment-411852263).

We do need a better story about conditional/version-specific instrumentation?  see #86.

this PR reverts #82 and adds node 10.0.0 to the build matrix (which fails without this revert)

toshok: cc @garthk 

Patch:
diff --git a/.eslintrc.json b/.eslintrc.json
index 899a4ae..434c02a 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -4,7 +4,7 @@
   },
   "extends": ["eslint:recommended"],
   "parserOptions": {
-    "ecmaVersion": 2017
+    "ecmaVersion": 2015
   },
   "rules": {
     "no-console": "off",
diff --git a/.travis.yml b/.travis.yml
index 75e50c6..9f041a8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -2,6 +2,7 @@ language: node_js
 node_js:
   - 8.9.1
   - "9"
+  - 10.0.0
   - "10"
 deploy:
   provider: npm
diff --git a/lib/instrumentation.js b/lib/instrumentation.js
index 551f302..9ce9c22 100644
--- a/lib/instrumentation.js
+++ b/lib/instrumentation.js
@@ -1,5 +1,6 @@
 /* eslint-env node */
 const shimmer = require("shimmer"),
+  tracker = require("./async_tracker"),
   Module = require("module"),
   process = require("process"),
   path = require("path"),
@@ -28,6 +29,29 @@ const instrumentationPath = name => {
   return `${__dirname}/instrumentation/${name}`;
 };
 
+let preloadDone = false;
+const instrumentPreload = (exports.instrumentPreload = () => {
+  if (preloadDone) {
+    return;
+  }
+
+  preloadDone = true;
+  // this really shouldn't be here.  we could load the instrumentation modules all at configure time, and implement this as an instrumentation
+  // that isn't invoked in response to a require (instead running at startup.)
+  shimmer.wrap(global.Promise.prototype, "then", function(original) {
+    return function then(onFulfilled, onRejected) {
+      let args = [];
+      if (arguments.length > 0) {
+        args.push(tracker.bindFunction(onFulfilled));
+      }
+      if (arguments.length > 1) {
+        args.push(tracker.bindFunction(onRejected));
+      }
+      return original.apply(this, args);
+    };
+  });
+});
+
 // arguments are the same as to require.resolve.
 function getPackageVersion(request, options) {
   // treat `require("react-dom/server")` like `require("react-dom")` here
@@ -130,6 +154,7 @@ exports.configure = (opts = {}) => {
       };
     });
   }
+  instrumentPreload();
 };
 
 exports.clearInstrumentationForTesting = () => {


Test patch:
diff --git a/lib/async_tracker.test.js b/lib/async_tracker.test.js
index 4761ce7..6af9b4f 100644
--- a/lib/async_tracker.test.js
+++ b/lib/async_tracker.test.js
@@ -46,42 +46,3 @@ test("callbacks are automatically attached to their parent context's payload", d
   expect(run).toBe(false);
   tracker.deleteTracked();
 });
-
-test("async/await chains with inbuilt Promises propagate the payload", done => {
-  const payload = {};
-  tracker.setTracked(payload);
-
-  chain().then(done, done);
-
-  async function chain() {
-    await delay();
-    await check();
-  }
-
-  async function delay() {
-    await new Promise(resolve => setTimeout(resolve, 100));
-  }
-
-  async function check() {
-    expect(tracker.getTracked()).toBe(payload);
-    tracker.deleteTracked();
-  }
-});
-
-test("manual chains with inbuilt Promises propagate the payload", done => {
-  const payload = {};
-  tracker.setTracked(payload);
-
-  delay()
-    .then(check)
-    .then(done, done);
-
-  function delay() {
-    return new Promise(resolve => setTimeout(resolve, 100));
-  }
-
-  function check() {
-    expect(tracker.getTracked()).toBe(payload);
-    tracker.deleteTracked();
-  }
-});
diff --git a/lib/instrumentation.test.js b/lib/instrumentation.test.js
index 4bd318e..1bd9c13 100644
--- a/lib/instrumentation.test.js
+++ b/lib/instrumentation.test.js
@@ -1,6 +1,12 @@
 /* eslint-env node, jest */
 const instrumentation = require("./instrumentation");
 
+test("preload shims Promise.then", () => {
+  expect(Promise.prototype.then.__wrapped).toBeUndefined();
+  instrumentation.instrumentPreload();
+  expect(Promise.prototype.then.__wrapped).toBe(true);
+});
+
 let createFakeExpress = () => {
   return {};
 };