fix(supabase_flutter): Safely check if conn is not null to avoid null check operator (#1178)

* safely check if realtime.conn is not null to avoid null check operator used on null value

* fix: ensure disconnecting state is followed by disconnected

* test: add tests for disconnect behavior

---------

Co-authored-by: Vinzent <[email protected]>

Author: martinpelli

packages/realtime_client/lib/src/realtime_client.dart

@@ -222,18 +222,22 @@ class RealtimeClient {
     final conn = this.conn;
     if (conn != null) {
       final oldState = connState;
-      connState = SocketStates.disconnecting;
-      log('transport', 'disconnecting', {'code': code, 'reason': reason},
-          Level.FINE);
+      final shouldCloseSink =
+          oldState == SocketStates.open || oldState == SocketStates.connecting;
+      if (shouldCloseSink) {
+        // Don't set the state to `disconnecting` if the connection is already closed.
+        connState = SocketStates.disconnecting;
+        log('transport', 'disconnecting', {'code': code, 'reason': reason},
+            Level.FINE);
+      }
 
       // Connection cannot be closed while it's still connecting. Wait for connection to
       // be ready and then close it.
       if (oldState == SocketStates.connecting) {
         await conn.ready.catchError((_) {});
       }
 
-      if (oldState == SocketStates.open ||
-          oldState == SocketStates.connecting) {
+      if (shouldCloseSink) {
         if (code != null) {
           await conn.sink.close(code, reason ?? '');
         } else {
@@ -319,7 +323,7 @@ class RealtimeClient {
     }
   }
 
-  /// Retuns `true` is the connection is open.
+  /// Returns `true` is the connection is open.
   bool get isConnected => connState == SocketStates.open;
 
   /// Removes a subscription from the socket.

packages/realtime_client/test/socket_test.dart

@@ -51,11 +51,15 @@ void main() {
 
   setUp(() async {
     mockServer = await HttpServer.bind('localhost', 0);
+    WebSocketChannel? channel;
+
     mockServer.transform(WebSocketTransformer()).listen((webSocket) {
-      final channel = IOWebSocketChannel(webSocket);
-      channel.stream.listen((request) {
-        channel.sink.add(request);
+      channel = IOWebSocketChannel(webSocket);
+      channel!.stream.listen((request) {
+        channel!.sink.add(request);
       });
+    }, onDone: () {
+      channel?.sink.close();
     });
   });
 
@@ -170,11 +174,15 @@ void main() {
       socket.disconnect();
     });
 
-    test('establishes websocket connection with endpoint', () {
-      socket.connect();
+    test('establishes websocket connection with endpoint', () async {
+      final connFuture = socket.connect();
+      expect(socket.connState, SocketStates.connecting);
 
       final conn = socket.conn;
 
+      await connFuture;
+      expect(socket.connState, SocketStates.open);
+
       expect(conn, isA<IOWebSocketChannel>());
       //! Not verifying connection url
     });
@@ -239,9 +247,11 @@ void main() {
 
     test('removes existing connection', () async {
       await socket.connect();
+
+      expect(socket.conn, isNotNull);
       await socket.disconnect();
 
-      expect(socket.conn, null);
+      expect(socket.conn, isNull);
     });
 
     test('calls callback', () async {
@@ -284,6 +294,36 @@ void main() {
       ).called(1);
     });
 
+    test('disconnecting a closed connections stays closed', () async {
+      await socket.connect();
+      expect(socket.connState, SocketStates.open);
+      await mockServer.close();
+      await Future.delayed(const Duration(milliseconds: 200));
+      expect(socket.connState, SocketStates.closed);
+      expect(socket.conn, isNotNull);
+
+      final disconnectFuture = socket.disconnect();
+
+      // `connState` stays `closed` during disconnect
+      expect(socket.connState, SocketStates.closed);
+      await disconnectFuture;
+      expect(socket.connState, SocketStates.closed);
+      expect(socket.conn, isNull);
+    });
+
+    test('disconnecting an open connection', () async {
+      await socket.connect();
+      expect(socket.connState, SocketStates.open);
+
+      final disconnectFuture = socket.disconnect();
+
+      // `connState` stays `closed` during disconnect
+      expect(socket.connState, SocketStates.disconnecting);
+      await disconnectFuture;
+      expect(socket.connState, SocketStates.disconnected);
+      expect(socket.conn, isNull);
+    });
+
     test('does not throw when no connection', () {
       expect(() => socket.disconnect(), returnsNormally);
     });

packages/supabase_flutter/lib/src/supabase.dart

@@ -237,7 +237,8 @@ class Supabase with WidgetsBindingObserver {
   Future<void> onResumed() async {
     final realtime = Supabase.instance.client.realtime;
     if (realtime.channels.isNotEmpty) {
-      if (realtime.connState == SocketStates.disconnecting) {
+      if (realtime.connState == SocketStates.disconnecting &&
+          realtime.conn != null) {
         // If the socket is still disconnecting from e.g.
         // [AppLifecycleState.paused] we should wait for it to finish before
         // reconnecting.