Fix reconnection issue

[ms42]

Changes:

• Fix timer thread, so that it won't die on an exception and keeps firing ClientReconnecting or ClientDisconnected events (in rare occasions connectionState() throws a NullPointerException)
• Add safe guard when using isRunning() for either creating or destroying variable timer
• Add settings parameter disconnectOnTcpChannelError to switch off immediate disconnection in case of a channel error, like a connection reset by the server (which happens quite often on a reconnect using es v4).

This should fix issue #29. Testing it with the following configuration and having no issues so far:

EventStoreBuilder.newBuilder()
                    .singleNodeAddress("127.0.0.1", 1113)
                    .userCredentials("admin", "changeit")
                    .maxOperationRetries(-1)
                    .maxReconnections(-1)
                    .disconnectOnTcpChannelError(false)
                    .build();

[ms42]

Integrationtests work fine locally, I assume the following error is a load issue?

java.util.concurrent.CompletionException: com.github.msemys.esjc.projection.ProjectionException: Server returned 408 (Server was unable to handle request in time) for POST on /projection/projection-ITProjectionManager-resetsProjection[0]-39c73c90-d785-4714-aa81-47eb76becd30/command/reset
Caused by: com.github.msemys.esjc.projection.ProjectionException: Server returned 408 (Server was unable to handle request in time) for POST on /projection/projection-ITProjectionManager-resetsProjection[0]-39c73c90-d785-4714-aa81-47eb76becd30/command/reset

Is there a way to easily re-run the tests?

[msemys]

Thanks for PR!

I really liked connect/disconnect and timer fixes. It's a good idea to have disconnectOnTcpChannelError switch at the moment, because I was thinking about to remove that .whenChannelError(EventStoreTcp.this::onChannelError) but I need more testing on this.

Could you resolve the conflict in EventStoreTcp.java?

about ProjectionException: Server returned 408 (Server was unable to handle request in time) - it's likely related to load and environment where es is running. I was playing with es v5 a couple of days and my first impression is that integration tests with es v5 runs much stable, than with es v4.

[ms42]

With changing the default behaviour from true to false for disconnectOnTcpChannelError two integration tests fail:

1. ITSslCertificateConnection..failsWithNonMatchingCertificateFile
2. ITSslCommonNameConnection.failsWithNonMatchingCommonName

First guess without taking a deeper look into the failing tests is that the tests are running into a timeout as the connection is not disconnected immediately and the client keeps trying to reconnect.

After taking a look at the implementation I noticed a small difference compared to the previous implementation. When disconnecting the implementation will emit an error event, i.e. fireEvent(Events.errorOccurred(throwable));, but without disconnecting this event won't be fired.

Maybe it's a good idea to always fire such an event, like

    private void onChannelError(Throwable throwable) {
        if(settings().disconnectOnTcpChannelError) {
            handle(new CloseConnection("Error when processing TCP package", throwable));
        } else {
            logger.debug("Failed processing TCP package", throwable);
            fireEvent(Events.errorOccurred(throwable));
        }
    }

I updated the implementation according to this and ITs are fine. I'm using log level debug again as the error might be handled by an event listener (and it matches the behaviour on client disconnect).

[msemys]

:+1: thanks!