Handle push errors in CommunicationProblemHandler (#11733)

Change-Id: I685a03df9bb785ed7c1c672c9b9cbf47609711e7

5 files changed, 105 insertions, 33 deletions

diff --git a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
index 45095bedb1..72f39b87c1 100644
--- a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
+++ b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
@@ -27,7 +27,6 @@ import com.vaadin.client.ApplicationConfiguration;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.ApplicationConnection.ApplicationStoppedEvent;
 import com.vaadin.client.ApplicationConnection.ApplicationStoppedHandler;
-import com.vaadin.client.ApplicationConnection.CommunicationErrorHandler;
 import com.vaadin.client.ResourceLoader;
 import com.vaadin.client.ResourceLoader.ResourceLoadEvent;
 import com.vaadin.client.ResourceLoader.ResourceLoadListener;
@@ -127,8 +126,6 @@ public class AtmospherePushConnection implements PushConnection {
 
     private String transport;
 
-    private CommunicationErrorHandler errorHandler;
-
     /**
      * Keeps track of the disconnect confirmation command for cases where
      * pending messages should be pushed before actually disconnecting.
@@ -147,10 +144,8 @@ public class AtmospherePushConnection implements PushConnection {
      */
     @Override
     public void init(final ApplicationConnection connection,
-            final PushConfigurationState pushConfiguration,
-            CommunicationErrorHandler errorHandler) {
+            final PushConfigurationState pushConfiguration) {
         this.connection = connection;
-        this.errorHandler = errorHandler;
 
         connection.addHandler(ApplicationStoppedEvent.TYPE,
                 new ApplicationStoppedHandler() {
@@ -279,8 +274,9 @@ public class AtmospherePushConnection implements PushConnection {
      * @since 7.2
      */
     protected void onConnect(AtmosphereResponse response) {
-        transport = response.getTransport();
 
+        transport = response.getTransport();
+        getCommunicationProblemHandler().pushOk(this);
         switch (state) {
         case CONNECT_PENDING:
             state = State.CONNECTED;
@@ -361,32 +357,27 @@ public class AtmospherePushConnection implements PushConnection {
      */
     protected void onError(AtmosphereResponse response) {
         state = State.DISCONNECTED;
-        errorHandler.onError("Push connection using "
-                + getConfig().getTransport() + " failed!",
-                response.getStatusCode());
+        getCommunicationProblemHandler().pushError(this);
     }
 
     protected void onClose(AtmosphereResponse response) {
         getLogger().info("Push connection closed");
         state = State.CONNECT_PENDING;
+        getCommunicationProblemHandler().pushClosed(this);
     }
 
     protected void onClientTimeout(AtmosphereResponse response) {
         state = State.DISCONNECTED;
-        errorHandler
-                .onError(
-                        "Client unexpectedly disconnected. Ensure client timeout is disabled.",
-                        -1);
+        getCommunicationProblemHandler().pushClientTimeout(this);
     }
 
     protected void onReconnect(JavaScriptObject request,
             final AtmosphereResponse response) {
         if (state == State.CONNECTED) {
-            getLogger()
-                    .fine("No onClose was received before reconnect. Forcing state to closed.");
             state = State.CONNECT_PENDING;
         }
         getLogger().info("Reopening push connection");
+        getCommunicationProblemHandler().pushReconnectPending(this);
     }
 
     public static abstract class AbstractJSO extends JavaScriptObject {
@@ -557,10 +548,8 @@ public class AtmospherePushConnection implements PushConnection {
 
                         @Override
                         public void onError(ResourceLoadEvent event) {
-                            errorHandler.onError(
-                                    event.getResourceUrl()
-                                            + " could not be loaded. Push will not work.",
-                                    0);
+                            getCommunicationProblemHandler()
+                                    .pushScriptLoadError(event.getResourceUrl());
                         }
                     });
         }
@@ -591,4 +580,9 @@ public class AtmospherePushConnection implements PushConnection {
     private static Logger getLogger() {
         return Logger.getLogger(AtmospherePushConnection.class.getName());
     }
+
+    private CommunicationProblemHandler getCommunicationProblemHandler() {
+        return connection.getCommunicationProblemHandler();
+    }
+
 }
diff --git a/client/src/com/vaadin/client/communication/CommunicationProblemHandler.java b/client/src/com/vaadin/client/communication/CommunicationProblemHandler.java
index 9c580fb6d9..50fb4605b0 100644
--- a/client/src/com/vaadin/client/communication/CommunicationProblemHandler.java
+++ b/client/src/com/vaadin/client/communication/CommunicationProblemHandler.java
@@ -277,4 +277,88 @@ public class CommunicationProblemHandler {
                 "Exception sending heartbeat: " + exception.getMessage());
         return true;
     }
+
+    /**
+     * @since
+     * @param response
+     */
+    public void pushError(PushConnection pushConnection) {
+        /*
+         * Push error indicates a fatal error we cannot (or should not) recover
+         * from by reconnecting or similar.
+         * 
+         * Atmosphere calls onError
+         * 
+         * 1. if maxReconnectAttemps is reached
+         * 
+         * 2. if neither the transport nor the fallback transport can be used
+         * 
+         * (also for server response codes != 200 if reconnectOnServerError is
+         * set to false but we do not set that to false)
+         */
+        connection.handleCommunicationError("Push connection using "
+                + pushConnection.getTransportType() + " failed!", -1);
+    }
+
+    /**
+     * @since
+     * @param response
+     */
+    public void pushClientTimeout(PushConnection pushConnection) {
+        connection
+                .handleCommunicationError(
+                        "Client unexpectedly disconnected. Ensure client timeout is disabled.",
+                        -1);
+
+    }
+
+    /**
+     * @since
+     * @param resourceUrl
+     */
+    public void pushScriptLoadError(String resourceUrl) {
+        connection.handleCommunicationError(resourceUrl
+                + " could not be loaded. Push will not work.", 0);
+
+    }
+
+    /**
+     * @since
+     */
+    public void heartbeatOk() {
+        getLogger().fine("Heartbeat response OK");
+    }
+
+    /**
+     * @since
+     */
+    public void xhrOk() {
+
+    }
+
+    /**
+     * @since
+     * @param response
+     */
+    public void pushClosed(PushConnection pushConnection) {
+        // TODO Auto-generated method stub
+
+    }
+
+    /**
+     * @since
+     * @param response
+     */
+    public void pushReconnectPending(PushConnection pushConnection) {
+        // TODO Auto-generated method stub
+
+    }
+
+    /**
+     * @since
+     */
+    public void pushOk(PushConnection pushConnection) {
+        // TODO Auto-generated method stub
+
+    }
 }
diff --git a/client/src/com/vaadin/client/communication/Heartbeat.java b/client/src/com/vaadin/client/communication/Heartbeat.java
index c8353f3215..eb39622a1a 100644
--- a/client/src/com/vaadin/client/communication/Heartbeat.java
+++ b/client/src/com/vaadin/client/communication/Heartbeat.java
@@ -97,7 +97,7 @@ public class Heartbeat {
 
                 boolean reschedule = true;
                 if (status == Response.SC_OK) {
-                    getLogger().fine("Heartbeat response OK");
+                    connection.getCommunicationProblemHandler().heartbeatOk();
                 } else {
                     reschedule = connection.getCommunicationProblemHandler()
                             .heartbeatInvalidStatusCode(request, response);
diff --git a/client/src/com/vaadin/client/communication/PushConnection.java b/client/src/com/vaadin/client/communication/PushConnection.java
index 8066746dc6..e0ea5a7d66 100644
--- a/client/src/com/vaadin/client/communication/PushConnection.java
+++ b/client/src/com/vaadin/client/communication/PushConnection.java
@@ -18,8 +18,8 @@ package com.vaadin.client.communication;
 
 import com.google.gwt.user.client.Command;
 import com.vaadin.client.ApplicationConnection;
-import com.vaadin.client.ApplicationConnection.CommunicationErrorHandler;
 import com.vaadin.shared.ui.ui.UIState.PushConfigurationState;
+
 import elemental.json.JsonObject;
 
 /**
@@ -41,8 +41,7 @@ public interface PushConnection {
      *            The ApplicationConnection
      */
     public void init(ApplicationConnection connection,
-            PushConfigurationState pushConfigurationState,
-            CommunicationErrorHandler errorHandler);
+            PushConfigurationState pushConfigurationState);
 
     /**
      * Pushes a message to the server. Will throw an exception if the connection
diff --git a/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java b/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java
index d961d59372..a53e74d2bd 100644
--- a/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java
+++ b/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java
@@ -32,7 +32,6 @@ import com.google.gwt.user.client.Window.ClosingEvent;
 import com.google.gwt.user.client.Window.ClosingHandler;
 import com.vaadin.client.ApplicationConfiguration;
 import com.vaadin.client.ApplicationConnection;
-import com.vaadin.client.ApplicationConnection.CommunicationErrorHandler;
 import com.vaadin.client.ApplicationConnection.RequestStartingEvent;
 import com.vaadin.client.ApplicationConnection.ResponseHandlingEndedEvent;
 import com.vaadin.client.BrowserInfo;
@@ -252,7 +251,9 @@ public class ServerCommunicationHandler {
 
                 int statusCode = response.getStatusCode();
 
-                if (statusCode != 200) {
+                if (statusCode == 200) {
+                    getCommunicationProblemHandler().xhrOk();
+                } else {
                     // There was a problem
                     CommunicationProblemEvent problemEvent = new CommunicationProblemEvent(
                             request, uri, payload, response);
@@ -368,13 +369,7 @@ public class ServerCommunicationHandler {
 
         if (enabled && push == null) {
             push = GWT.create(PushConnection.class);
-            push.init(connection, pushState, new CommunicationErrorHandler() {
-                @Override
-                public boolean onError(String details, int statusCode) {
-                    connection.handleCommunicationError(details, statusCode);
-                    return true;
-                }
-            });
+            push.init(connection, pushState);
         } else if (!enabled && push != null && push.isActive()) {
             push.disconnect(new Command() {
                 @Override