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.

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

Warning spamming in Transport.onclose #538

Open
ak-slongchamps opened this issue Dec 14, 2020 · 2 comments · May be fixed by #539
Open

Warning spamming in Transport.onclose #538

ak-slongchamps opened this issue Dec 14, 2020 · 2 comments · May be fixed by #539

Comments

@ak-slongchamps
Copy link

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 added a commit to ak-slongchamps/autobahn-js that referenced this issue Dec 14, 2020
@ak-slongchamps ak-slongchamps linked a pull request Dec 14, 2020 that will close this issue
@kuema
Copy link

kuema commented Feb 16, 2021

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".

log.warn("auto-reconnecting in " + next_retry.delay + "s ..");

@ak-slongchamps
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants