From 93e93b1693c294dd5873d5840fe23c83cfe7a84b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A1bio=20Henriques?= <contact@fabiohenriques.dev>
Date: Tue, 27 Aug 2024 16:26:12 +0100
Subject: [PATCH] Improve runtime safety checks in connection manager functions

---
 lib/OnyxConnectionManager.ts            | 16 ++++++++--
 tests/unit/OnyxConnectionManagerTest.ts | 39 ++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts
index cee40880..d73cb30f 100644
--- a/lib/OnyxConnectionManager.ts
+++ b/lib/OnyxConnectionManager.ts
@@ -227,8 +227,14 @@ class OnyxConnectionManager {
      * Adds the connection to the eviction block list. Connections added to this list can never be evicted.
      * */
     addToEvictionBlockList(connection: Connection): void {
-        const connectionMetadata = this.connectionsMap.get(connection?.id);
+        if (!connection) {
+            Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list passing an undefined connection object.`);
+            return;
+        }
+
+        const connectionMetadata = this.connectionsMap.get(connection.id);
         if (!connectionMetadata) {
+            Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list but no connection was found.`);
             return;
         }
 
@@ -245,8 +251,14 @@ class OnyxConnectionManager {
      * which will enable it to be evicted again.
      */
     removeFromEvictionBlockList(connection: Connection): void {
-        const connectionMetadata = this.connectionsMap.get(connection?.id);
+        if (!connection) {
+            Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list passing an undefined connection object.`);
+            return;
+        }
+
+        const connectionMetadata = this.connectionsMap.get(connection.id);
         if (!connectionMetadata) {
+            Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list but no connection was found.`);
             return;
         }
 
diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts
index a9f77a3c..d6d4e26b 100644
--- a/tests/unit/OnyxConnectionManagerTest.ts
+++ b/tests/unit/OnyxConnectionManagerTest.ts
@@ -1,13 +1,14 @@
 // eslint-disable-next-line max-classes-per-file
 import {act} from '@testing-library/react-native';
 import Onyx from '../../lib';
+import type {Connection} from '../../lib/OnyxConnectionManager';
 import connectionManager from '../../lib/OnyxConnectionManager';
+import OnyxUtils from '../../lib/OnyxUtils';
 import StorageMock from '../../lib/storage';
 import type {OnyxKey, WithOnyxConnectOptions} from '../../lib/types';
 import type {WithOnyxInstance} from '../../lib/withOnyx/types';
 import type GenericCollection from '../utils/GenericCollection';
 import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
-import OnyxUtils from '../../lib/OnyxUtils';
 
 // We need access to `connectionsMap` and `generateConnectionID` during the tests but the properties are private,
 // so this workaround allows us to have access to them.
@@ -270,6 +271,18 @@ describe('OnyxConnectionManager', () => {
             expect(connectionsMap.size).toEqual(0);
         });
 
+        it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => {
+            expect(connectionsMap.size).toEqual(0);
+
+            expect(() => {
+                connectionManager.disconnect(undefined as unknown as Connection);
+            }).not.toThrow();
+
+            expect(() => {
+                connectionManager.disconnect({id: 'connectionID1', callbackID: 'callbackID1'});
+            }).not.toThrow();
+        });
+
         describe('withOnyx', () => {
             it('should connect to a key two times with withOnyx and create two connections instead of one', async () => {
                 await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test');
@@ -390,5 +403,29 @@ describe('OnyxConnectionManager', () => {
             // inexistent connection ID, shouldn't do anything
             expect(() => connectionManager.removeFromEvictionBlockList({id: 'connectionID0', callbackID: 'callbackID0'})).not.toThrow();
         });
+
+        it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside addToEvictionBlockList()', () => {
+            expect(connectionsMap.size).toEqual(0);
+
+            expect(() => {
+                connectionManager.addToEvictionBlockList(undefined as unknown as Connection);
+            }).not.toThrow();
+
+            expect(() => {
+                connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'});
+            }).not.toThrow();
+        });
+
+        it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside removeFromEvictionBlockList()', () => {
+            expect(connectionsMap.size).toEqual(0);
+
+            expect(() => {
+                connectionManager.removeFromEvictionBlockList(undefined as unknown as Connection);
+            }).not.toThrow();
+
+            expect(() => {
+                connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'});
+            }).not.toThrow();
+        });
     });
 });