Merge branch 'fix-stuck-disconnecting'

3 files changed, 23 insertions, 59 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 294bd5a905..2b754c4d29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,6 +38,8 @@ Line wrap the file at 100 chars.                                              Th
 ### Fixed
 - Fix duplicate "Connected"/"Disconnected" desktop notifications caused by the daemon sending
   multiple consecutive tunnel state events for the same state.
+- Fix GUI appearing stuck in "Disconnecting" state when daemon transitions directly from error to
+  disconnected.
 - Fix QUIC obfuscation not always being used if relays only had IPv6 addresses for QUIC.
 - Fix a bug with Shadowsocks-based API access methods where some ciphers were configurable by
   Mullvad VPN clients while not being supported by the system service.
diff --git a/desktop/packages/mullvad-vpn/src/main/tunnel-state.ts b/desktop/packages/mullvad-vpn/src/main/tunnel-state.ts
index 5ca3365136..95e3403754 100644
--- a/desktop/packages/mullvad-vpn/src/main/tunnel-state.ts
+++ b/desktop/packages/mullvad-vpn/src/main/tunnel-state.ts
@@ -35,6 +35,8 @@ export default class TunnelStateHandler {
 
   // This function sets a new tunnel state as an assumed next state and saves the current state as
   // fallback. The fallback is used if the assumed next state isn't reached.
+  // The timeout is needed to move out of the "predicted" state in case an actual state is never
+  // emitted.
   public expectNextTunnelState(state: 'connecting' | 'disconnecting') {
     this.tunnelStateFallback = this.tunnelState;
 
@@ -53,23 +55,15 @@ export default class TunnelStateHandler {
   }
 
   public handleNewTunnelState(newState: TunnelState) {
-    // If there's a fallback state set then the app is in an assumed next state and need to check
-    // if it's now reached or if the current state should be ignored and set as the fallback state.
+    // Remove fallback state since we know the real state now
     if (this.tunnelStateFallback) {
-      if (this.tunnelState.state === newState.state || newState.state === 'error') {
-        this.tunnelStateFallbackScheduler.cancel();
-        this.tunnelStateFallback = undefined;
-      } else {
-        this.tunnelStateFallback = newState;
-        return;
-      }
+      this.tunnelStateFallbackScheduler.cancel();
+      this.tunnelStateFallback = undefined;
     }
 
     if (newState.state === 'disconnecting' && newState.details === 'reconnect') {
-      // When reconnecting there's no need of showing the disconnecting state. This switches to the
-      // connecting state immediately.
-      this.expectNextTunnelState('connecting');
-      this.tunnelStateFallback = newState;
+      // The reconnecting state should appear the same as the connecting state.
+      this.setTunnelState({ state: 'connecting', featureIndicators: undefined });
     } else {
       if (newState.state === 'disconnected' && newState.location !== undefined) {
         this.lastKnownDisconnectedLocation = newState.location;
diff --git a/desktop/packages/mullvad-vpn/test/unit/tunnel-state.spec.ts b/desktop/packages/mullvad-vpn/test/unit/tunnel-state.spec.ts
index 28995e58d3..517b10b5b9 100644
--- a/desktop/packages/mullvad-vpn/test/unit/tunnel-state.spec.ts
+++ b/desktop/packages/mullvad-vpn/test/unit/tunnel-state.spec.ts
@@ -20,83 +20,51 @@ describe('Tunnel state', () => {
     tunnelStateHandler.handleNewTunnelState(connecting);
     tunnelStateHandler.handleNewTunnelState(error);
     tunnelStateHandler.handleNewTunnelState(disconnected);
+    tunnelStateHandler.handleNewTunnelState(connected);
 
-    expect(stateUpdateSpy).toHaveBeenCalledTimes(4);
+    expect(stateUpdateSpy).toHaveBeenCalledTimes(5);
     expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'disconnecting');
     expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'connecting');
     expect(stateUpdateSpy).toHaveBeenNthCalledWith(3, 'error');
     expect(stateUpdateSpy).toHaveBeenNthCalledWith(4, 'disconnected');
-    expect(tunnelStateHandler.tunnelState.state).toEqual('disconnected');
-  });
-
-  it('Should ignore non-expected state update', () => {
-    const stateUpdateSpy = vi.fn();
-    const handleTunnelStateUpdate = (tunnelState: TunnelState) => stateUpdateSpy(tunnelState.state);
-    const tunnelStateHandler = new TunnelStateHandler({ handleTunnelStateUpdate });
-
-    tunnelStateHandler.expectNextTunnelState('connecting');
-    tunnelStateHandler.handleNewTunnelState(disconnecting);
-    tunnelStateHandler.handleNewTunnelState(connecting);
-
-    expect(stateUpdateSpy).toHaveBeenCalledTimes(2);
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'connecting');
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'connecting');
-    expect(tunnelStateHandler.tunnelState.state).toEqual('connecting');
-  });
-
-  it('Should allow new states after expected state is reached', () => {
-    const stateUpdateSpy = vi.fn();
-    const handleTunnelStateUpdate = (tunnelState: TunnelState) => stateUpdateSpy(tunnelState.state);
-    const tunnelStateHandler = new TunnelStateHandler({ handleTunnelStateUpdate });
-
-    tunnelStateHandler.expectNextTunnelState('connecting');
-    tunnelStateHandler.handleNewTunnelState(disconnected);
-    tunnelStateHandler.handleNewTunnelState(connecting);
-    tunnelStateHandler.handleNewTunnelState(connected);
-
-    expect(stateUpdateSpy).toHaveBeenCalledTimes(3);
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'connecting');
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'connecting');
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(3, 'connected');
+    expect(stateUpdateSpy).toHaveBeenNthCalledWith(5, 'connected');
     expect(tunnelStateHandler.tunnelState.state).toEqual('connected');
   });
 
-  it('Should allow error state update', () => {
+  it('Should accept any real state while expecting a predicted state', () => {
     const stateUpdateSpy = vi.fn();
     const handleTunnelStateUpdate = (tunnelState: TunnelState) => stateUpdateSpy(tunnelState.state);
     const tunnelStateHandler = new TunnelStateHandler({ handleTunnelStateUpdate });
 
-    tunnelStateHandler.expectNextTunnelState('connecting');
-    tunnelStateHandler.handleNewTunnelState(disconnected);
     tunnelStateHandler.handleNewTunnelState(error);
+    tunnelStateHandler.expectNextTunnelState('disconnecting');
     tunnelStateHandler.handleNewTunnelState(disconnected);
 
     expect(stateUpdateSpy).toHaveBeenCalledTimes(3);
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'connecting');
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'error');
+    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'disconnecting');
     expect(stateUpdateSpy).toHaveBeenNthCalledWith(3, 'disconnected');
     expect(tunnelStateHandler.tunnelState.state).toEqual('disconnected');
   });
 
-  it('Should time out and use last ignored state', () => {
+  it('Should time out and use last known state', () => {
     vi.useFakeTimers();
     const stateUpdateSpy = vi.fn();
     const handleTunnelStateUpdate = (tunnelState: TunnelState) => stateUpdateSpy(tunnelState.state);
     const tunnelStateHandler = new TunnelStateHandler({ handleTunnelStateUpdate });
 
-    tunnelStateHandler.expectNextTunnelState('connecting');
     tunnelStateHandler.handleNewTunnelState(disconnected);
-    tunnelStateHandler.handleNewTunnelState(connected);
+    tunnelStateHandler.expectNextTunnelState('connecting');
 
-    expect(stateUpdateSpy).toHaveBeenCalledTimes(1);
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'connecting');
+    expect(stateUpdateSpy).toHaveBeenCalledTimes(2);
+    expect(stateUpdateSpy).toHaveBeenNthCalledWith(1, 'disconnected');
+    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'connecting');
     expect(tunnelStateHandler.tunnelState.state).toEqual('connecting');
 
     vi.advanceTimersByTime(3000);
 
-    expect(stateUpdateSpy).toHaveBeenCalledTimes(2);
-    expect(stateUpdateSpy).toHaveBeenNthCalledWith(2, 'connected');
-    expect(tunnelStateHandler.tunnelState.state).toEqual('connected');
+    expect(stateUpdateSpy).toHaveBeenCalledTimes(3);
+    expect(stateUpdateSpy).toHaveBeenNthCalledWith(3, 'disconnected');
+    expect(tunnelStateHandler.tunnelState.state).toEqual('disconnected');
 
     vi.useRealTimers();
   });