THRIFT-4288: Implement logging levels in node.js properly
Client: nodejs
This closes #1334
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 @@
}
// 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 @@
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;
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 @@
// 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;
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 @@
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 @@
});
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 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');
});
}
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;
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');
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);
};
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 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 @@
frame = result.nextFrame;
}
} catch(e) {
- console.log("TWebSocketTransport Exception: " + e);
+ log.error('TWebSocketTransport Exception: ' + e);
socket.destroy();
}
});
@@ -556,9 +557,3 @@
//Return the server
return server;
};
-
-
-
-
-
-
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.__onError = function(evt) {
- console.log("Thrift WebSocket Error: " + evt.toString());
+ log.error('websocket: ' + evt.toString());
this.socket.close();
};