lib: support different loggers

[lundibundi]

Enables users to specify logger they want to use with our library. Also adds simple proxy to console.

As discussed in https://github.com/metarhia/jstp/issues/102 I extract logger fuctionality to a different PR.

Also I'd like to add a few tests for console logger, but I don't know how to better implement it, as I don't think that either redirecting console output during the test or just checking stdout is a good solution, can someone suggest a solution?

[nechaido]

I'd like the logger to have method's like: warn, error, log, debug. Logger should decide for itself to log something or not, that way we'll have no need to write this kind of statements: if (logger.isDebug()) logPacket(packet);.

[lundibundi]

@nechaido It already has those methods and decides by itself whether he should log or drop the message. The need in methods like isDebug() and such arises from the cost that preparation for logging has, for example in this PR I have to check jstp message type and copy if necessary while removing some fields to avoid leak of private data to logs (for handshake packet). Checking isDebug() is much cheaper than that. And also I don't think that implementing callback to check current logging level is that hard.

[nechaido]

@lundibundi this could be achieved by passing a function to logger which will do the preparation and then return the string to be logged.

[belochub]

@lundibundi, can't you make a wrapper (adapter) in our library which will check the logger level before the preparation, to avoid boilerplate checking like the one @nechaido mentioned?

[lundibundi]

@belochub I can do that but I see no reason for that complexity, whether we write if (logger.isDebug()) logPacket(packet); or something like logger.log('debug', logPacket.bind(null, packet)) More so the latter will probably be less efficient and takes more work to implement and support.

[nechaido]

@lundibundi and what about:

logger.debug(() => preparePacket(packet));

Looks ok for me and doesn't do any unnecessary preparation in release mod, only call to empty function.

Forgot about creating a function to be passed.

[belochub]

@lundibundi, what you said in https://github.com/metarhia/jstp/pull/224#issuecomment-310670911 wasn't at all what I proposed to do.

[belochub]

@lundibundi, what I proposed will make

if (logger.isDebug()) logPacket(packet);

look like

logger.get().debug(packet);

or even

logger.debug(packet);

[lundibundi]

@belochub oh, sorry, I thought that you extended @nechaido idea. So you want a logger specifically to log jstp messages, that will check log level and if object is passed treat it as jstp message and log it same way logPacket does and if string is passed log it as is, is my understanding correct?

[belochub]

@lundibundi, yeah, something like that.

[lundibundi]

@belochub but its only usage are those 3 calls in connection.js and I'm not sure if it is plausible to implement it solely for those usages? Also I can move isDebug check into logPacket so there'll be only one check for isDebug() (in code), they only reason I left it outside was to avoid unnecessary call to logPacket.

[aqrln]

@lundibundi ping

[lundibundi]

@aqrln I'm waiting for the decision of our team, whether this is a good solution or not. We even got the slack vote which has apparently been flooded by github bot.

[belochub]

Closed in favor of https://github.com/metarhia/jstp/pull/252 (see https://github.com/metarhia/jstp/issues/250 for more information).