Merge pull request #1836 from uProxy/bemasc-retry

Add auto-retry on connection failure.

src/chrome/extension/scripts/chrome_core_connector.spec.ts

@@ -172,23 +172,23 @@ describe('core-connector', () => {
 
   it('show disconnect.html if user was proxying when app disconnects.', (done) => {
     var uiIsGettingAccessSpy = spyOn(ui, 'isGettingAccess');
-    var uiStopGettingInUiAndConfigSpy = spyOn(ui, 'stopGettingInUiAndConfig');
+    var uiStoppedGettingSpy = spyOn(ui, 'stoppedGetting');
     connectToApp().then(() => {
       spyOn(chromeCoreConnector, 'connect').and.callFake(() => { done(); });
       uiIsGettingAccessSpy.and.callFake(() => { return true; });
       disconnect();
-      expect(uiStopGettingInUiAndConfigSpy).toHaveBeenCalled();
+      expect(uiStoppedGettingSpy).toHaveBeenCalled();
     });
   });
 
   it('do not show disconnect.html if user was not proxying when app disconnects.', (done) => {
     var uiIsGettingAccessSpy = spyOn(ui, 'isGettingAccess');
-    var uiStopGettingInUiAndConfigSpy = spyOn(ui, 'stopGettingInUiAndConfig');
+    var uiStoppedGettingSpy = spyOn(ui, 'stoppedGetting');
     connectToApp().then(() => {
       spyOn(chromeCoreConnector, 'connect').and.callFake(() => { done(); });
       uiIsGettingAccessSpy.and.callFake(() => { return false; });
       disconnect();
-      expect(uiStopGettingInUiAndConfigSpy).not.toHaveBeenCalled();
+      expect(uiStoppedGettingSpy).not.toHaveBeenCalled();
     });
   });
 

src/generic_ui/locales/en/messages.json

@@ -392,8 +392,8 @@
     "message": "Oops! You've been disconnected from your friend."
   },
   "DISCONNECTED_MESSAGE": {
-    "description": "Instruction informing the user that to continue browsing the Internet, the user will need to use their local (potentially unsafe and/or censored) connection.",
-    "message": "Please proceed with caution. Your web traffic will no longer be routed through your friend. You may want to close any sensitive windows you have open, before proceeding."
+    "description": "Instruction informing the user that if they want to browse the Internet and stop waiting for reconnection, the user will need to use their local (potentially unsafe and/or censored) connection.",
+    "message": "If you disable the proxy, your web traffic will no longer be routed through your friend. You may want to close any sensitive pages you have open, before proceeding."
   },
   "CONTINUE_BROWSING": {
     "description": "Label for button that will change the users Internet settings back to using the user's local connection (i.e. not their friend's Internet).",
@@ -675,9 +675,17 @@
     "description": "Shown in Chrome to instruct the user to install part 2 of uProxy.",
     "message": "Download and enable part 2 of uProxy to get started."
   },
+  "RECONNECTING": {
+    "description": "Indicate that uProxy is attempting to reconnect automatically after a failure.  A progress indicator is also visible.",
+    "message": "Reconnecting..."
+  },
+  "RECONNECT_FAILED": {
+    "description": "Indicate that uProxy attempted to reconnect automatically but it failed.",
+    "message": "Reconnect failed"
+  },
   "RESTART_PROXYING": {
-    "description": "If proxy connection gets disconnected restart the connection",
-    "message": "Reconnect"
+    "description": "Displayed on a button next to the RECONNECT_FAILED message",
+    "message": "Try again"
   },
   "UPDATE_AVAILABLE": {
     "description": "What to show the user (on the bottom bar) when an update is available.  The actual update will not be done until they restart uProxy which we will prompt them to do with a link.",

src/generic_ui/polymer/instance.ts

@@ -29,6 +29,7 @@ Polymer({
     });
   },
   stop: function() {
+    ui.stopUsingProxy();
     ui.stopGettingFromInstance(this.instance.instanceId);
   }
 });

src/generic_ui/polymer/root.html

@@ -190,6 +190,9 @@
       #disconnectDialog paper-button {
         font-size: 12px;
       }
+      #progressWrapper {
+        text-align: center;
+      }
       paper-toast {
         position: fixed;
         left: 12px;
@@ -371,16 +374,25 @@ <h3>{{ "WELCOME" | $$ }}</h3>
     </uproxy-action-dialog>
 
     <uproxy-action-dialog id='disconnectDialog' backdrop layered="false"
-        opened="{{ core.disconnectedWhileProxying }}" autoCloseDisabled>
+        opened="{{ !!core.disconnectedWhileProxying }}" autoCloseDisabled>
       <img src='../icons/128_error.png'>
       <div id="disconnectDialogText">
         <h1>{{ "DISCONNECTED_TITLE" | $$ }}</h1>
+        <div id='progressWrapper'>
+          <div hidden?='{{ !ui.instanceTryingToGetAccessFrom }}'>
+            <strong>{{ 'RECONNECTING' | $$ }}</strong>
+            <paper-progress indeterminate='true'></paper-progress>
+          </div>
+          <div hidden?='{{ !!ui.instanceTryingToGetAccessFrom }}' >
+            <strong>{{ 'RECONNECT_FAILED' | $$ }}</strong>
+            <paper-button class='dialogButton' on-tap='{{ restartProxying }}'>{{ 'RESTART_PROXYING' | $$ }}</paper-button>
+          </div>
+        </div>
         <p>
           {{ "DISCONNECTED_MESSAGE" | $$ }}
         </p>
       </div>
       <paper-button class='dialogButton' on-tap='{{ revertProxySettings }}'>{{ "CONTINUE_BROWSING" | $$ }}</paper-button>
-      <paper-button class='dialogButton' on-tap='{{ restartProxying }}'>{{ 'RESTART_PROXYING' | $$ }}</paper-button>
     </uproxy-action-dialog>
 
     <uproxy-troubleshoot titleText='{{ troubleshootTitle }}'></uproxy-troubleshoot>

src/generic_ui/polymer/root.ts

@@ -167,7 +167,7 @@ Polymer({
     }
   },
   revertProxySettings: function() {
-    this.ui.stopGettingInUiAndConfig({instanceId: null, error: false});
+    this.ui.stopUsingProxy();
   },
   restartProxying: function() {
     this.ui.restartProxying();

src/generic_ui/scripts/core_connector.ts

@@ -30,7 +30,9 @@ class CoreConnector implements uproxy_core_api.CoreApi {
   private mapPromiseIdToFulfillAndReject_ :{[id :number] : FullfillAndReject} =
       {};
 
-  public disconnectedWhileProxying = false;
+  // If non-null, the ID of the instance from which we are presently
+  // disconnected.
+  public disconnectedWhileProxying :string = null;
 
   constructor(private browserConnector_ :browser_connector.CoreBrowserConnector) {
     this.browserConnector_.onUpdate(uproxy_core_api.Update.COMMAND_FULFILLED,

src/generic_ui/scripts/ui.spec.ts

@@ -22,7 +22,15 @@ describe('UI.UserInterface', () => {
     // Create a fresh UI object before each test.
     mockCore = jasmine.createSpyObj(
         'core',
-        ['reset', 'onUpdate', 'sendCommand', 'on', 'connect', 'getFullState']);
+        [
+          'reset',
+          'onUpdate',
+          'sendCommand',
+          'on',
+          'connect',
+          'getFullState',
+          'stop'
+        ]);
 
     // assume connect always resolves immediately
     (<jasmine.Spy>mockCore.connect).and.returnValue(Promise.resolve());
@@ -248,7 +256,7 @@ describe('UI.UserInterface', () => {
           'testInstanceId', { address : 'testAddress' , port : 0 });
       expect(mockBrowserApi.setIcon)
           .toHaveBeenCalledWith(Constants.GETTING_ICON);
-      ui.stopGettingInUiAndConfig({instanceId: null, error: false});
+      ui.stoppedGetting({instanceId: null, error: false});
     });
 
     it('Extension icon changes when you stop getting access', () => {

src/generic_ui/scripts/ui.ts

@@ -191,9 +191,6 @@ export class UserInterface implements ui_constants.UiApi {
   // ID of the most recent failed proxying attempt.
   public proxyingId: string;
 
-  // is a proxy currently set
-  private proxySet_ :boolean = false;
-
   // Must be included in Chrome extension manifest's list of permissions.
   public AWS_FRONT_DOMAIN = 'https://a0.awsstatic.com/';
 
@@ -246,7 +243,7 @@ export class UserInterface implements ui_constants.UiApi {
       this.view = ui_constants.View.BROWSER_ERROR;
 
       if (this.isGettingAccess()) {
-        this.stopGettingInUiAndConfig({instanceId: null, error: true});
+        this.stoppedGetting({instanceId: null, error: true});
       }
     });
 
@@ -283,7 +280,7 @@ export class UserInterface implements ui_constants.UiApi {
 
     // indicates the current getting connection has ended
     core.onUpdate(uproxy_core_api.Update.STOP_GETTING, (error :boolean) => {
-      this.stopGettingInUiAndConfig({instanceId: null, error: error});
+      this.stoppedGetting({instanceId: null, error: error});
     });
 
     // indicates we just started offering access through copy+paste
@@ -308,7 +305,7 @@ export class UserInterface implements ui_constants.UiApi {
 
     core.onUpdate(uproxy_core_api.Update.STOP_GETTING_FROM_FRIEND,
         (data :social.StopProxyInfo) => { // TODO better type
-        this.stopGettingInUiAndConfig(data);
+        this.stoppedGetting(data);
     });
 
     core.onUpdate(uproxy_core_api.Update.START_GIVING_TO_FRIEND,
@@ -369,11 +366,16 @@ export class UserInterface implements ui_constants.UiApi {
         (info:uproxy_core_api.FailedToGetOrGive) => {
       console.error('proxying attempt ' + info.proxyingId + ' failed (getting)');
 
-      this.toastMessage = this.i18n_t("UNABLE_TO_GET_FROM", {
-        name: info.name
-      });
+      if (!this.core.disconnectedWhileProxying) {
+        // This is an immediate failure, i.e. failure of a connection attempt
+        // that never connected.  It is not a retry.
+        // Show the error toast indicating that a get attempt failed.
+        this.toastMessage = this.i18n_t("UNABLE_TO_GET_FROM", {
+          name: info.name
+        });
+        this.unableToGet = true;
+      }
       this.instanceTryingToGetAccessFrom = null;
-      this.unableToGet = true;
       this.proxyingId = info.proxyingId;
       this.bringUproxyToFront();
     });
@@ -573,35 +575,49 @@ export class UserInterface implements ui_constants.UiApi {
   }
 
   /**
-   * Removes proxy indicators from UI and undoes proxy configuration
-   * (e.g. chrome.proxy settings).
+   * Takes all actions required when getting stops, including removing proxy
+   * indicators from the UI, and retrying the connection if appropriate.
    * If user didn't end proxying, so if proxy session ended because of some
    * unexpected reason, user should be asked before reverting proxy settings.
    * if data.instanceId is null, it means to stop active proxying.
    */
-  public stopGettingInUiAndConfig = (data :social.StopProxyInfo) => {
-    if (data.instanceId) {
-      this.mapInstanceIdToUser_[data.instanceId].isSharingWithMe = false;
-    } else if (this.instanceGettingAccessFrom_) {
-      this.mapInstanceIdToUser_[this.instanceGettingAccessFrom_].isSharingWithMe = false;
+  public stoppedGetting = (data :social.StopProxyInfo) => {
+    var instanceId = data.instanceId || this.instanceGettingAccessFrom_;
+
+    if (instanceId === this.instanceGettingAccessFrom_) {
+      this.instanceGettingAccessFrom_ = null;
     }
 
-    if (data.error) {
-      this.bringUproxyToFront();
-      this.core.disconnectedWhileProxying = true;
-    } else {
-      this.core.disconnectedWhileProxying = false;
-      if (data.instanceId === null ||
-          data.instanceId === this.instanceGettingAccessFrom_) {
-        this.instanceGettingAccessFrom_ = null;
-        this.browserApi.stopUsingProxy();
+    if (instanceId) {
+      this.mapInstanceIdToUser_[instanceId].isSharingWithMe = false;
+      if (data.error) {
+        this.bringUproxyToFront();
+        this.core.disconnectedWhileProxying = instanceId;
+        // Auto-retry.
+        this.restartProxying();
       }
     }
 
     this.updateGettingStatusBar_();
     this.updateIcon_();
   }
 
+  /**
+   * Undoes proxy configuration (e.g. chrome.proxy settings).
+   */
+  public stopUsingProxy = () => {
+    this.browserApi.stopUsingProxy();
+    this.core.disconnectedWhileProxying = null;
+    this.updateIcon_();
+
+    // revertProxySettings might call stopUsingProxy while a reconnection is
+    // still being attempted.  In that case, we also want to terminate the
+    // in-progress connection.
+    if (this.instanceTryingToGetAccessFrom) {
+      this.stopGettingFromInstance(this.instanceTryingToGetAccessFrom);
+    }
+  }
+
   private getInstancePath_ = (instanceId :string) => {
     var user = this.mapInstanceIdToUser_[instanceId];
 
@@ -616,7 +632,7 @@ export class UserInterface implements ui_constants.UiApi {
   }
 
   public restartProxying = () => {
-    this.startGettingFromInstance(this.instanceGettingAccessFrom_);
+    this.startGettingFromInstance(this.core.disconnectedWhileProxying);
   }
 
   public startGettingFromInstance = (instanceId :string) :Promise<void> => {
@@ -632,6 +648,9 @@ export class UserInterface implements ui_constants.UiApi {
         this.core.stop(this.getInstancePath_(this.instanceGettingAccessFrom_));
       }
       this.startGettingInUiAndConfig(instanceId, endpoint);
+    }, (err:Error) => {
+      this.instanceTryingToGetAccessFrom = null;
+      throw err;
     });
   }
 
@@ -663,7 +682,7 @@ export class UserInterface implements ui_constants.UiApi {
       this.mapInstanceIdToUser_[instanceId].isSharingWithMe = true;
     }
 
-    this.core.disconnectedWhileProxying = false;
+    this.core.disconnectedWhileProxying = null;
 
     this.startGettingInUi();
 
@@ -754,7 +773,7 @@ export class UserInterface implements ui_constants.UiApi {
           this.reconnect(networkMsg.name);
         } else {
           if (this.instanceGettingAccessFrom_) {
-            this.stopGettingInUiAndConfig({instanceId: null, error: true});
+            this.stopGettingFromInstance(this.instanceGettingAccessFrom_);
           }
           this.showNotification(this.i18n_t("LOGGED_OUT", {network: networkMsg.name}));
 
@@ -817,12 +836,17 @@ export class UserInterface implements ui_constants.UiApi {
     }
 
     for (var i = 0; i < payload.offeringInstances.length; i++) {
-      if (payload.offeringInstances[i].localGettingFromRemote ===
-          social.GettingState.GETTING_ACCESS) {
-        this.instanceGettingAccessFrom_ = payload.offeringInstances[i].instanceId;
+      var gettingState = payload.offeringInstances[i].localGettingFromRemote;
+      var instanceId = payload.offeringInstances[i].instanceId;
+      if (gettingState === social.GettingState.GETTING_ACCESS) {
+        this.instanceGettingAccessFrom_ = instanceId;
         user.isSharingWithMe = true;
         this.updateGettingStatusBar_();
         break;
+      } else if (gettingState === social.GettingState.TRYING_TO_GET_ACCESS) {
+        this. instanceTryingToGetAccessFrom = instanceId;
+        this.updateGettingStatusBar_();
+        break;
       }
     }