fix: there is no uv_handle_type_name in earlier versions of libuv

PR-URL: https://github.com/hyj1991/xprofiler/pull/45 Reviewed-By: hyj1991 <yeekwanvong@gmail.com>
X-Profiler · Jan 8, 2020 · d0cf7c1 · d0cf7c1
1 parent 42344e8
commit d0cf7c1
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 29 deletions.
diff --git a/package.json b/package.json
@@ -10,9 +10,9 @@
     "build": "npm run lint && node-gyp rebuild",
     "format": "clang-format -i --glob=\"src/**/!(report_win)[.h|.cc]\"",
     "lint": "npm run format && eslint --fix xprofiler.js \"test/**/*.js\" lib/*.js patch/*.js bin/xprofctl scripts/**/*.js",
-    "test": "mocha -t 0 -R spec test/*.test.js",
+    "test": "mocha -t 0 -R spec test/*.test.js test/patch/*.test.js",
     "test-single": "mocha -t 0 -R spec",
-    "cov": "nyc --reporter=html --reporter=text --reporter=lcov mocha -R spec test/*.test.js  --timeout 0",
+    "cov": "nyc --reporter=html --reporter=text --reporter=lcov mocha -R spec test/*.test.js test/patch/*.test.js --timeout 0",
     "cov-single": "nyc --reporter=html --reporter=text --reporter=lcov mocha --timeout 0 -R spec",
     "upload": "node scripts/upload.js",
     "ci": "npm run lint && npm run cov && codecov && npm run upload",

diff --git a/patch/http.js b/patch/http.js
@@ -26,13 +26,12 @@ function serverWrapper(addLiveRequest, addCloseRequest, addSentRequest, original
 
     if (typeof opts === 'function') {
       args.splice(0, 1, requestListenerWrapper(opts, addLiveRequest, addCloseRequest, addSentRequest));
-      returned = original.apply(this, args);
-    }
-    if (typeof requestListener === 'function') {
+    } else if (typeof requestListener === 'function') {
       args.splice(1, 1, requestListenerWrapper(requestListener, addLiveRequest, addCloseRequest, addSentRequest));
-      returned = original.apply(this, args);
     }
 
+    returned = original.apply(this, args);
+
     return returned;
   };
 }

diff --git a/src/commands/parser.cc b/src/commands/parser.cc
@@ -13,12 +13,13 @@ using nlohmann::json;
 using std::exception;
 using std::string;
 
-#define HANDLE_COMMANDS(cmd_str, handle)                                 \
-  if (strcmp(cmd.c_str(), #cmd_str) == 0) {                              \
-    handle(parsed, FmtMessage,                                           \
-           [traceid](json data) { SuccessValue(traceid, data); },        \
-           [traceid](string message) { ErrorValue(traceid, message); }); \
-    handled = true;                                                      \
+#define HANDLE_COMMANDS(cmd_str, handle)                              \
+  if (strcmp(cmd.c_str(), #cmd_str) == 0) {                           \
+    handle(                                                           \
+        parsed, FmtMessage,                                           \
+        [traceid](json data) { SuccessValue(traceid, data); },        \
+        [traceid](string message) { ErrorValue(traceid, message); }); \
+    handled = true;                                                   \
   }
 
 void ParseCmd(char *command) {

diff --git a/src/commands/report/uv_statistics.cc b/src/commands/report/uv_statistics.cc
@@ -227,43 +227,60 @@ static void reportEndpoints(uv_handle_t *h, ostringstream &out) {
 
 static void walkHandle(uv_handle_t *h, void *arg) {
   uv_any_handle *handle = reinterpret_cast<uv_any_handle *>(h);
+
   ostringstream data;
+  // const char *type = uv_handle_type_name(h->type);
+  string type;
 
   switch (h->type) {
     case UV_UNKNOWN_HANDLE:
+      type = "unknown";
       break;
     case UV_ASYNC:
+      type = "async";
       break;
     case UV_CHECK:
+      type = "check";
       break;
     case UV_FS_EVENT: {
+      type = "fs_event";
       reportPath(h, data);
       break;
     }
     case UV_FS_POLL: {
+      type = "fs_poll";
       reportPath(h, data);
       break;
     }
     case UV_HANDLE:
+      type = "handle";
       break;
     case UV_IDLE:
+      type = "idle";
       break;
     case UV_NAMED_PIPE:
+      type = "pipe";
       break;
     case UV_POLL:
+      type = "poll";
       break;
     case UV_PREPARE:
+      type = "prepare";
       break;
     case UV_PROCESS:
+      type = "process";
       data << "pid: " << handle->process.pid;
       break;
     case UV_STREAM:
+      type = "stream";
       break;
     case UV_TCP: {
+      type = "tcp";
       reportEndpoints(h, data);
       break;
     }
     case UV_TIMER: {
+      type = "timer";
 #if defined(_WIN32) && (UV_VERSION_HEX < ((1 << 16) | (22 << 8)))
       uint64_t due = handle->timer.due;
 #else
@@ -279,6 +296,7 @@ static void walkHandle(uv_handle_t *h, void *arg) {
       break;
     }
     case UV_TTY: {
+      type = "tty";
       int height, width, rc;
       rc = uv_tty_get_winsize(&(handle->tty), &width, &height);
       if (rc == 0) {
@@ -287,17 +305,21 @@ static void walkHandle(uv_handle_t *h, void *arg) {
       break;
     }
     case UV_UDP: {
+      type = "udp";
       reportEndpoints(h, data);
       break;
     }
     case UV_SIGNAL: {
+      type = "signal";
       data << "signum: " << handle->signal.signum << " ("
            << SignoString(handle->signal.signum) << ")";
       break;
     }
     case UV_FILE:
+      type = "file";
       break;
     case UV_HANDLE_TYPE_MAX:
+      type = "max";
       break;
   }
 
@@ -349,7 +371,6 @@ static void walkHandle(uv_handle_t *h, void *arg) {
   }
 
   JSONWriter *writer = static_cast<JSONWriter *>(arg);
-  const char *type = uv_handle_type_name(h->type);
   string detail = data.str();
 
   writer->json_start();

diff --git a/src/logbypass/http.cc b/src/logbypass/http.cc
@@ -1,7 +1,7 @@
-#include "uv.h"
+#include "http.h"
 
 #include "../logger.h"
-#include "http.h"
+#include "uv.h"
 
 namespace xprofiler {
 using Nan::To;

diff --git a/test/fixtures/non-blocking.js b/test/fixtures/non-blocking.js
@@ -24,15 +24,11 @@ xprofiler.setHooks();
 xprofiler.setHooks();
 
 // http server
-const server1 = http.createServer(function (req, res) {
+const server = http.createServer(function (req, res) {
   setTimeout(() => res.end('hello world.'), 100);
 });
-server1.listen(8443, () => console.log('http server listen at 8443...'));
-server1.unref();
-
-const server2 = http.createServer({}, function (req, res) { res.end('hello world.'); });
-server2.listen(9443, () => console.log('http server listen at 9443...'));
-server2.unref();
+server.listen(8443, () => console.log('http server listen at 8443...'));
+server.unref();
 
 function sendRequest(abort) {
   const req = http.request('http://localhost:8443');
@@ -62,8 +58,7 @@ setTimeout(() => {
   clearInterval(interval);
   console.log('will close...');
   setTimeout(() => {
-    server1.close();
-    server2.close();
+    server.close();
     console.log('closed');
   }, 200);
 }, 8000);
diff --git a/test/patch/http.test.js b/test/patch/http.test.js
@@ -0,0 +1,96 @@
+'use strict';
+
+const http = require('http');
+const EventEmitter = require('events').EventEmitter;
+const mm = require('mm');
+const expect = require('expect.js');
+const { patchHttp } = require('../../patch/http');
+
+
+describe(`patch http.createServer(cb)`, function () {
+  const requestTimes = 5;
+  let triggerTimes = 0;
+
+  let liveRequest = 0;
+  let closeRequest = 0;
+  let sentRequest = 0;
+
+  function addLiveRequest() {
+    liveRequest++;
+  }
+
+  function addCloseRequest() {
+    closeRequest++;
+  }
+
+  function addSentRequest() {
+    sentRequest++;
+  }
+
+  function mockCreateServer(opts, requestHandle) {
+    return new Promise(resolve => {
+      let times = 0;
+      const interval = setInterval(() => {
+        if (times < requestTimes) {
+          const request = new EventEmitter();
+          const response = new EventEmitter();
+          if (typeof opts === 'function') {
+            opts(request, response);
+          } else if (typeof requestHandle === 'function') {
+            requestHandle(request, response);
+          }
+          times++;
+        } else {
+          clearInterval(interval);
+          resolve();
+        }
+      }, 100);
+    });
+  }
+
+  before(async function () {
+    mm(http, 'createServer', mockCreateServer);
+    patchHttp(addLiveRequest, addCloseRequest, addSentRequest);
+    await http.createServer(function (request, response) {
+      triggerTimes++;
+      response.emit('finish');
+      response.emit('close');
+    });
+
+    await http.createServer({}, function (request, response) {
+      triggerTimes++;
+      response.emit('finish');
+      response.emit('close');
+    });
+
+    await http.createServer({}, {}, function (request, response) {
+      triggerTimes++;
+      response.emit('finish');
+      response.emit('close');
+    });
+  });
+
+  after(function () {
+    mm.restore();
+  });
+
+  it('patch should be ok', function () {
+    expect(http.createServer).not.to.be(mockCreateServer);
+  });
+
+  it(`request handler should trigger ${requestTimes} * 2 times`, function () {
+    expect(triggerTimes).to.be(requestTimes * 2);
+  });
+
+  it(`live request shoule be ${requestTimes} * 2`, function () {
+    expect(liveRequest).to.be(requestTimes * 2);
+  });
+
+  it(`close request shoule be ${requestTimes} * 2`, function () {
+    expect(closeRequest).to.be(requestTimes * 2);
+  });
+
+  it(`sent request shoule be ${requestTimes} * 2`, function () {
+    expect(sentRequest).to.be(requestTimes * 2);
+  });
+});
diff --git a/test/shimmer.test.js → test/patch/shimmer.test.js b/test/shimmer.test.js → test/patch/shimmer.test.js
@@ -1,7 +1,7 @@
 'use strict';
 
 const expect = require('expect.js');
-const { wrap, unwrap } = require('../patch/shimmer');
+const { wrap, unwrap } = require('../../patch/shimmer');
 
 describe('wrap / unwrap module', function () {
   it('wrap module failed with wrong params', function () {

diff --git a/test/start.test.js b/test/start.test.js
@@ -51,8 +51,8 @@ describe(`xprofiler starting`, function () {
         expect(aliveProcess[1]).to.be(logdir);
       });
 
-      it(`.xprofiler cwd: ${aliveProcess[2]} should be ${/^([.\w()/\\:]+|)$/}`, function () {
-        expect(/^([.\w()/\\:]+|)$/.test(aliveProcess[2])).to.be.ok();
+      it(`.xprofiler cwd: ${aliveProcess[2]} should be ${/^([.\w()/\\:-]+|)$/}`, function () {
+        expect(/^([.\w()/\\:-]+|)$/.test(aliveProcess[2])).to.be.ok();
       });
 
       it(`.xprofiler executable: ${aliveProcess[3]} should be node-${process.version}`, function () {
@@ -64,8 +64,8 @@ describe(`xprofiler starting`, function () {
         expect(version).to.be(process.version);
       });
 
-      it(`.xprofiler file: ${aliveProcess[4]} should be ${/^([.\w()/\\:]+|)$/}`, function () {
-        expect(/^([.\w()/\\:]+|)$/.test(aliveProcess[4])).to.be.ok();
+      it(`.xprofiler file: ${aliveProcess[4]} should be ${/^([.\w()/\\:-]+|)$/}`, function () {
+        expect(/^([.\w()/\\:-]+|)$/.test(aliveProcess[4])).to.be.ok();
       });
 
       it(`.xprofiler module path: ${aliveProcess[5]} should be ${path.join(__dirname, '..')}`, function () {