Merge pull request #580 from fabioh8010/feature/better-runtime-safety…

…-checks Improve runtime safety checks in connection manager functions
Expensify · Aug 27, 2024 · d0f4849 · d0f4849
2 parents e6e1b25 + 93e93b1
commit d0f4849
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 3 deletions.
diff --git 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
@@ -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();
+        });
     });
 });