autobahn-js icon indicating copy to clipboard operation
autobahn-js copied to clipboard

Warning spamming in Transport.onclose

Open ak-slongchamps opened this issue 3 years ago • 2 comments

Reproduction steps

  • Call autobahn.Connection.close on an active connection.
  • Observe the following is logged:
connection closed closed {
  reason: 'wamp.error.goodbye_and_out',
  message: '',
  retry_delay: null,
  retry_count: null,
  will_retry: false
}

This can be observed in the autobahn-js tests: https://travis-ci.org/github/crossbario/autobahn-js

✔ testConnect
connection closed closed { reason: 'wamp.close.normal',
  message: '',
  retry_delay: null,
  retry_count: null,
  will_retry: false }
CLOSE closed { reason: 'wamp.close.normal',
  message: '',
  retry_delay: null,
  retry_count: null,
  will_retry: false }
auto-reconnect disabled! false undefined
connection closed closed { reason: 'wamp.close.normal',
  message: '',
  retry_delay: null,
  retry_count: null,
  will_retry: false }

Tech Details

_session.leave is called in autobahn.Connection.close. Ultimately, Transport.onclose is being called when the connection is closed and this function logs a "connection closed {reason}" as a warning, when really this would only be a warning if the connection closed were a problematic state.

My suggestion is to simply use log.debug instead, as this spams stderr needlessly and can lead users to think that something went wrong on disconnection, when everything went well.

ak-slongchamps avatar Dec 14 '20 18:12 ak-slongchamps

I've noticed that as well. Since upgrading to a recent version of AutobahnJS, there's a lot more (IMO unnecessary) log activity. I think there's no need to log those reconnection attempts and disconnect reasons as warnings. I'd consider these as "normal" internal behaviour, when the connection was lost.

So I'd also reduce the log level of this line to "debug".

https://github.com/crossbario/autobahn-js/blob/eefb4e105f7c55da6331e583c60051c5fcdc0137/packages/autobahn/lib/connection.js#L371

kuema avatar Feb 16 '21 11:02 kuema

The current commit addresses warnings logged in the happy path only. As I mentioned in the ticket "this function logs a "connection closed {reason}" as a warning, when really this would only be a warning if the connection closed were a problematic state." Another issue could address this auto-reconnect logging specifically.

ak-slongchamps avatar Feb 17 '21 15:02 ak-slongchamps