Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve websocket logs #416

Open
wants to merge 1 commit into
base: old-dev
Choose a base branch
from
Open

chore: improve websocket logs #416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/websocket/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ abstract class BaseWebSocket extends EventEmitter {
this.heartbeat = setInterval(() => {
this.sendPing();
}, this.heartbeatInterval);

console.info('Websocket connection opened.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we are already logging the websocket ping timeout, but I don't like the idea of having logs on the lib, it pollutes the sdout of the clients using it

Maybe we should just emit an event here and log on the client

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we decide to emit an event to log something, I would suggest to create an util logger that centralizes all the logging on the lib and implements this event emit with the common interface of log, warn, error, etc.

That way we can have a familiar logging structure within the lib, just as if it was a common application, and have the centralized logger deal with the events. This would also make it easier for future logging configurations.

}

/**
Expand Down Expand Up @@ -277,7 +279,7 @@ abstract class BaseWebSocket extends EventEmitter {
* Event received when the websocket connection is down.
*/
onConnectionDown() {
console.warn('Ping timeout. Connection is down...', {
console.warn('Websocket ping timeout. Connection is down...', {
uptime: this.uptime(),
connectionTimeout: this.connectionTimeout,
});
Expand Down