You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2017/09/23 13:12:35 UTC

thrift git commit: THRIFT-4288: Implement logging levels in node.js properly Client: nodejs

Repository: thrift
Updated Branches:
  refs/heads/master aded00b61 -> c8e020705


THRIFT-4288: Implement logging levels in node.js properly
Client: nodejs

This closes #1334


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/c8e02070
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/c8e02070
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/c8e02070

Branch: refs/heads/master
Commit: c8e0207053a26f206d6515313747b7e1999a01d5
Parents: aded00b
Author: Equim <sa...@ekyu.moe>
Authored: Fri Aug 18 20:59:07 2017 +0800
Committer: James E. King, III <jk...@apache.org>
Committed: Sat Sep 23 06:11:34 2017 -0700

----------------------------------------------------------------------
 lib/nodejs/lib/thrift/binary_protocol.js  |  2 -
 lib/nodejs/lib/thrift/compact_protocol.js |  1 -
 lib/nodejs/lib/thrift/connection.js       | 22 ++++----
 lib/nodejs/lib/thrift/index.js            |  5 ++
 lib/nodejs/lib/thrift/json_protocol.js    |  1 -
 lib/nodejs/lib/thrift/log.js              | 73 +++++++++++++++++++++++---
 lib/nodejs/lib/thrift/web_server.js       |  9 +---
 lib/nodejs/lib/thrift/ws_transport.js     |  4 +-
 8 files changed, 86 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/binary_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/binary_protocol.js b/lib/nodejs/lib/thrift/binary_protocol.js
index 6d3918e..0c0ee50 100644
--- a/lib/nodejs/lib/thrift/binary_protocol.js
+++ b/lib/nodejs/lib/thrift/binary_protocol.js
@@ -55,7 +55,6 @@ TBinaryProtocol.prototype.writeMessageBegin = function(name, type, seqid) {
     }
     // Record client seqid to find callback again
     if (this._seqid) {
-      // TODO better logging log warning
       log.warning('SeqId already set', { 'name': name });
     } else {
       this._seqid = seqid;
@@ -177,7 +176,6 @@ TBinaryProtocol.prototype.readMessageBegin = function() {
   if (sz < 0) {
     var version = sz & VERSION_MASK;
     if (version != VERSION_1) {
-      console.log("BAD: " + version);
       throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.BAD_VERSION, "Bad version in readMessageBegin: " + sz);
     }
     type = sz & TYPE_MASK;

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/compact_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/compact_protocol.js b/lib/nodejs/lib/thrift/compact_protocol.js
index b0e9148..5c531e5 100644
--- a/lib/nodejs/lib/thrift/compact_protocol.js
+++ b/lib/nodejs/lib/thrift/compact_protocol.js
@@ -249,7 +249,6 @@ TCompactProtocol.prototype.writeMessageBegin = function(name, type, seqid) {
 
   // Record client seqid to find callback again
   if (this._seqid) {
-    // TODO better logging log warning
     log.warning('SeqId already set', { 'name': name });
   } else {
     this._seqid = seqid;

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/connection.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/connection.js b/lib/nodejs/lib/thrift/connection.js
index db2dbf6..273339b 100644
--- a/lib/nodejs/lib/thrift/connection.js
+++ b/lib/nodejs/lib/thrift/connection.js
@@ -17,11 +17,12 @@
  * under the License.
  */
 var util = require('util');
-var EventEmitter = require("events").EventEmitter;
+var EventEmitter = require('events').EventEmitter;
 var constants = require('constants');
 var net = require('net');
 var tls = require('tls');
 var thrift = require('./thrift');
+var log = require('./log');
 
 var TBufferedTransport = require('./buffered_transport');
 var TBinaryProtocol = require('./binary_protocol');
@@ -204,9 +205,7 @@ Connection.prototype.connection_gone = function () {
     this.retry_delay = Math.floor(this.retry_delay * this.retry_backoff);
   }
 
-  if (self._debug) {
-    console.log("Retry connection in " + this.retry_delay + " ms");
-  }
+  log.debug("Retry connection in " + this.retry_delay + " ms");
 
   if (this.max_attempts && this.attempts >= this.max_attempts) {
     this.retry_timer = null;
@@ -222,9 +221,7 @@ Connection.prototype.connection_gone = function () {
   });
 
   this.retry_timer = setTimeout(function () {
-    if (self._debug) {
-       console.log("Retrying connection...");
-  }
+    log.debug("Retrying connection...");
 
     self.retry_totaltime += self.retry_delay;
 
@@ -276,20 +273,19 @@ var StdIOConnection = exports.StdIOConnection = function(command, options) {
   var self = this;
   EventEmitter.call(this);
 
-  this._debug = options.debug || false;
   this.connection = child.stdin;
   this.options = options || {};
   this.transport = this.options.transport || TBufferedTransport;
   this.protocol = this.options.protocol || TBinaryProtocol;
   this.offline_queue = [];
 
-  if(this._debug === true){
-    this.child.stderr.on('data',function(err){
-      console.log(err.toString(),'CHILD ERROR');
+  if (log.getLogLevel() === 'debug') {
+    this.child.stderr.on('data', function (err) {
+      log.debug(err.toString(), 'CHILD ERROR');
     });
 
-    this.child.on('exit',function(code,signal){
-      console.log(code+':'+signal,'CHILD EXITED');
+    this.child.on('exit', function (code,signal) {
+      log.debug(code + ':' + signal, 'CHILD EXITED');
     });
   }
 

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/index.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/index.js b/lib/nodejs/lib/thrift/index.js
index 61e5cbd..020726d 100644
--- a/lib/nodejs/lib/thrift/index.js
+++ b/lib/nodejs/lib/thrift/index.js
@@ -18,6 +18,11 @@
  */
 exports.Thrift = require('./thrift');
 
+var log = require('./log');
+exports.setLogFunc = log.setLogFunc;
+exports.setLogLevel = log.setLogLevel;
+exports.getLogLevel = log.getLogLevel;
+
 var connection = require('./connection');
 exports.Connection = connection.Connection;
 exports.createClient = connection.createClient;

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/json_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js
index f31e3b2..84c62f2 100644
--- a/lib/nodejs/lib/thrift/json_protocol.js
+++ b/lib/nodejs/lib/thrift/json_protocol.js
@@ -17,7 +17,6 @@
  * under the License.
  */
 
-var log = require('./log');
 var Int64 = require('node-int64');
 var InputBufferUnderrunError = require('./transport').InputBufferUnderrunError;
 var Thrift = require('./thrift');

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/log.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/log.js b/lib/nodejs/lib/thrift/log.js
index 0e13ea8..053e813 100644
--- a/lib/nodejs/lib/thrift/log.js
+++ b/lib/nodejs/lib/thrift/log.js
@@ -17,10 +17,71 @@
  * under the License.
  */
 
-module.exports = {
-  'info' : function logInfo() {},
-  'warning' : function logWarning() {},
-  'error' : function logError() {},
-  'debug' : function logDebug() {},
-  'trace' : function logTrace() {}
+var util = require('util');
+
+var disabled = function () {};
+var logFunc = console.log;
+var logLevel = 'error'; // default level
+
+function factory(level) {
+  return function () {
+    // better use spread syntax, but due to compatibility,
+    // use legacy method here.
+    var args = ['thrift: [' + level + '] '].concat(Array.from(arguments));
+    return logFunc(util.format.apply(null, args));
+  };
+}
+
+var trace = disabled;
+var debug = disabled;
+var error = disabled;
+var warning = disabled;
+var info = disabled;
+
+exports.setLogFunc = function (func) {
+  logFunc = func;
+};
+
+var setLogLevel = exports.setLogLevel = function (level) {
+  trace = debug = error = warning = info = disabled;
+  logLevel = level;
+  switch (logLevel) {
+  case 'trace':
+    trace = factory('TRACE');
+  case 'debug':
+    debug = factory('DEBUG');
+  case 'error':
+    error = factory('ERROR');
+  case 'warning':
+    warning = factory('WARN');
+  case 'info':
+    info = factory('INFO');
+  }
+};
+
+// set default
+setLogLevel(logLevel);
+
+exports.getLogLevel = function () {
+  return logLevel;
+};
+
+exports.trace = function () {
+  return trace.apply(null, arguments);
+};
+
+exports.debug = function () {
+  return debug.apply(null, arguments);
+};
+
+exports.error = function () {
+  return error.apply(null, arguments);
+};
+
+exports.warning = function () {
+  return warning.apply(null, arguments);
+};
+
+exports.info = function () {
+  return info.apply(null, arguments);
 };

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/web_server.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/web_server.js b/lib/nodejs/lib/thrift/web_server.js
index 60b67b4..0093c8a 100644
--- a/lib/nodejs/lib/thrift/web_server.js
+++ b/lib/nodejs/lib/thrift/web_server.js
@@ -22,6 +22,7 @@ var url = require("url");
 var path = require("path");
 var fs = require("fs");
 var crypto = require("crypto");
+var log = require('./log');
 
 var MultiplexedProcessor = require('./multiplexed_processor').MultiplexedProcessor;
 
@@ -547,7 +548,7 @@ exports.createWebServer = function(options) {
           frame = result.nextFrame;
         }
       } catch(e) {
-        console.log("TWebSocketTransport Exception: " + e);
+        log.error('TWebSocketTransport Exception: ' + e);
         socket.destroy();
       }
     });
@@ -556,9 +557,3 @@ exports.createWebServer = function(options) {
   //Return the server
   return server;
 };
-
-
-
-
-
-

http://git-wip-us.apache.org/repos/asf/thrift/blob/c8e02070/lib/nodejs/lib/thrift/ws_transport.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/ws_transport.js b/lib/nodejs/lib/thrift/ws_transport.js
index 8e750e2..3513b84 100644
--- a/lib/nodejs/lib/thrift/ws_transport.js
+++ b/lib/nodejs/lib/thrift/ws_transport.js
@@ -17,6 +17,8 @@
  * under the License.
  */
 
+var log = require('./log');
+
 module.exports = TWebSocketTransport;
 
 /**
@@ -105,7 +107,7 @@ TWebSocketTransport.prototype.__onMessage = function(evt) {
 };
 
 TWebSocketTransport.prototype.__onError = function(evt) {
-  console.log("Thrift WebSocket Error: " + evt.toString());
+  log.error('websocket: ' + evt.toString());
   this.socket.close();
 };