Skip to content
This repository has been archived by the owner on May 22, 2019. It is now read-only.

log errors encountered #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

log errors encountered #61

wants to merge 1 commit into from

Conversation

santhosh-tekuri
Copy link
Contributor

if wamp serializer/deserializer throws any error, then state
remains in Connecting for-ever. no error in log. atleast
logging will give some clue to users

@Matthias247
Copy link
Owner

This should not be really needed.

There exist some contracts between the transport layer and the WAMP client/router logic layer - which unfortunatly haven't been documented - but which shoudl handle the situation correctly:

  • If the [de]serializer (which resides in the transport layer) encounters a protocol violation and/or throws an error the the transport layer shall close the connection. This means besides rejecting the promise for the current send it must also call IWampConnectionListener.transportClosed or IWampConnectionListener.transportError(error), which notifies the logic layer about the problem.
  • The transport layer must call any of those methods only once.
  • The logic layer will defer the evaluation of those methods into it's own EventLoop - this is handled by the IConnectionController logic, which is currently implemented as the QueueingConnectionController. The controller will take care the remaining WAMP related logic will be signaled that the connection was closed.
  • If the client gets informed that in the connecting state that the connection was closed it should move to other states (either close connection and retry later or close connection and move to the final state).
    It is intentional that the user won't be notified about that event because it would only result in a Disconnected->Connecting->Disconnected->Connecting->... event stream. Multiple connection attempts are signaled to the user as a single connection attempt - because signalling them individually won't bring any additional value to the user in most cases. E.g. the behavior convenionatly covers the "Network temporarily not available" or "Server temporarily not available" situations.
  • If the client gets informed in the connected state that the connection was closed it will mark all pending calls as failed and thereby inform the user (state => Disconnected).

If there is any issue where the current implementation does not match this described behavior it should be considered as a bug which really should be found and fixed - instead of logging the calls.

@santhosh-tekuri
Copy link
Contributor Author

i was bitten by this bug twice. when i was testing msgpack serialization on big-endian systems, there was a bug in msg-pack library. see msgpack/msgpack-java#266

to reproduce this: just add following line at WampDeserializationHandler:95

if(true) throw new RuntimeException("thrown intentionally")

now run ServerTest.java from examples. you will notice that program never ends. it simply gets stuck for ever. you dont see any exception at all
the client remains in "connenting" state for ever

if wamp serializer/deserializer throws any error, then state
remains in Connecting for-ever. no error in log. atleast
logging will give some clue to users
@Matthias247
Copy link
Owner

Hi I tested this and everything worked as intented. The throwed exception leads to the netty exception handlers being invoked, which will close the connection. Then the WampClient/StateController will get notified that the connection was closed. The internal state will then move from HandshakingState to WaitingForDisconnectState to WaitingForReconnectState to ConnectingState to HandshakingState and will try to Reconnect, just as I explained above. This means the clients internal state changes.

What doesn't change is the state that is propagated to the user. From his point of view the client is always in the Connecting state. The intentation was that if the client wants automatic reconnects it isn't interested in incomplete connection attempts and thereby disconnected -> connecting -> disconnected cycles. If you don't use infinite reconnects but a limited number then the statemachine will eventually move to Disconnected.
Or if you don't use automatic reconnects at all and instead build a new client on each connection failure.

If introducing logging would be helpful, then I would prefer to include logging for the state machine transisitions (either in the StateController or in the individual states) and not for the individual calls, because as I said the errors for those are already handled by the QueueingConnectionController.

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

Successfully merging this pull request may close these issues.

2 participants