WebexCommunity / webex-node-bot-framework

A framework for creating Webex Teams bots using node and express
MIT License
57 stars 48 forks source link

Remove console.log #104

Open neilord opened 4 months ago

neilord commented 4 months ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch webex-node-bot-framework@2.5.1 for the project I'm working on.

Please do not include console.logs in production libraries.

Here is the diff that solved my problem:

diff --git a/node_modules/webex-node-bot-framework/lib/websocket.js b/node_modules/webex-node-bot-framework/lib/websocket.js
index 96b65c9..91c70b3 100644
--- a/node_modules/webex-node-bot-framework/lib/websocket.js
+++ b/node_modules/webex-node-bot-framework/lib/websocket.js
@@ -58,11 +58,9 @@ Websocket.prototype.init = function() {
           this.framework.webex.memberships.on('updated', (event) => processEvent(this.framework, event, this.name));
           this.framework.webex.rooms.on('created', (event) => processEvent(this.framework, event, this.name));
           this.framework.webex.rooms.on('updated', (event) => processEvent(this.framework, event, this.name));
-          console.log('Listening for webex teams events...');
           return when(true);
         })
         .catch((err) => {
-          console.error(`error listening for webex teams events: ${err}`);
           return Promise.reject(err);
         });
     });
@@ -86,7 +84,6 @@ Websocket.prototype.cleanup = function() {
       this.framework.webex.memberships.off('updated');
       this.framework.webex.rooms.off('created');
       this.framework.webex.rooms.off('updated');
-      console.log('Stopped listening for webex teams events...');
       return when(val);
     }).catch(e => console.error(`Failed cleaning up websocket connection: ${e.message}`));
 };

This issue body was partially generated by patch-package.

jpjpjp commented 4 months ago

Hmm. What is the problem with console.log messages? Don't most prod environments have a way of managing these?

I'm not opposed to making (or approving contributed) changes, but it isn't clear to me what to replace it with. Feel free to join the user conversation happening in the webex-node-bot-framework Support Space to see if there is a community reccomendation.

neilord commented 4 months ago

@jpjpjp Thanks for the reply!

One of the reasons is that if the application uses a custom logging system, those technical console.logs from dependence packages can be undesired in the log files. It is considered a better practice to provide the solution below: It can be useful to have an option like logger that can be set to true to enable those console.logs.

Thanks!