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

Better reconnection handling #50

Open
KSDaemon opened this issue Oct 17, 2022 · 1 comment
Open

Better reconnection handling #50

KSDaemon opened this issue Oct 17, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@KSDaemon
Copy link
Collaborator

Copy pasted main points from #47 for context:

So in the first run, there are the next points of connection failure and reconnection:

  • Transport failures before session is established.
  • Wamp abort (due to auth or other reasons) before session is established. Wamp error means that server processed client messages and get to some point that resolves in abort. Does auto reconnection fix this? Seems that not.
  • Transport failures during session. Definitely the most important point to proceed with reconnection (if it is configured)
  • Wamp failures during session. Seems that this situations should not lead to reconnection. As they happens when something is broken on business logic side (like using feature that is not announced, using serializer that is not supported etc), so autoreconnection won't automatically fix them.
@konsultaner konsultaner added the bug Something isn't working label Oct 17, 2022
@konsultaner konsultaner self-assigned this Oct 24, 2022
@konsultaner
Copy link
Owner

@KSDaemon I tried to fix this issue, but then I realized some things.

A transport may have issues before and after the session creation. For example when a cluster is rolled out and the router is avalable after the client. The client would have to try to reconnect until the router is available. Also if the router for some reason crashes after the session creation and the client then reconnects after the router has been restarted.

Connectanum-dart only reconnects if transport layer issues appears. If the router kills the connection with an Abort, the client will not try to restart. This is done by having two Completer the onConnectionLost and onDisconnect. Only onConnectionLost will make the client try to reconnect. The Session and the Client object will only call the onDisconnect or transport.close() if something on the wamp-layer happens.

I had to fix one issue. When adding the wamp.error.no_such_role, wamp.error.no_such_topic and the wamp.error.no_such_session authentication error, one must have missed to add it to the authentication failures in the client.dart. I will add this and push it later.

So I think:

Transport failures before session is established

Transport failures during session. Definitely the most important point to proceed with reconnection (if it is configured)

Should lead to a reconnect, if configured

Wamp abort (due to auth or other reasons) before session is established. Wamp error means that server processed client messages and get to some point that resolves in abort. Does auto reconnection fix this? Seems that not.

Wamp failures during session. Seems that this situations should not lead to reconnection. As they happens when something is broken on business logic side (like using feature that is not announced, using serializer that is not supported etc), so autoreconnection won't automatically fix them.

This should be configurable. The client currently has an Error map that will decide if the client will try to reconnect on auth problems. This will only happen once. There is no second retry. Imagine a situation where a client tries to authenticate, but the database is not up or down at the moment. The router must fail the authentication and it might do so with a lie to hide the original reason. If the client tries to reconnect and the database is available again, the authentication would succeed the second time. So I guess even this scenario should be configurable.

if (![
Error.notAuthorized,
Error.noPrincipal,
Error.authorizationFailed,
Error.noSuchRealm,
Error.protocolViolation
].contains(abort.reason) &&
options.reconnectTime != null) {
// if the router restarts we should wait until it has been initialized
await Future.delayed(Duration(seconds: 2));
options.reconnectCount = 0;
_connect(options);
} else {

If a session has been esteblished an Abort will lead into a transport.close() and thus into a onDisconnect which will not start a reconnect. Due to the WAMP protocol definition a protocol violation will send an Abort. An Error should not be a reason for a connection loss.

} else if (message is Abort) {
try {
transport.close();
} catch (ignore) {/* my be already closed */}
welcomeCompleter.completeError(message);
} else if (message is Goodbye) {

So what do you think?

I think I should make it more configurable. Add a reconnectOnAuthError:bool and a authErrorForReconnectList:array or something like this. And maybe have a special handler that will be notifyed on Abort. The individual client may decide wether to reconnect or not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants