chore(web): catch and log errors when building connection info instead of throwing COMPASS-8848

packages/compass-web/src/connection-storage.spec.ts

@@ -1,5 +1,9 @@
 import { expect } from 'chai';
-import { buildConnectionInfoFromClusterDescription } from './connection-storage';
+import { createNoopLogger } from '@mongodb-js/compass-logging/provider';
+import {
+  buildConnectionInfoFromClusterDescription,
+  AtlasCloudConnectionStorage,
+} from './connection-storage';
 import type { ClusterDescriptionWithDataProcessingRegion } from './connection-storage';
 
 const deployment = {
@@ -140,7 +144,7 @@ describe('buildConnectionInfoFromClusterDescription', function () {
         connectionString
       );
 
-      expect(connectionInfo.connectionOptions.lookup()).to.deep.eq({
+      expect(connectionInfo.connectionOptions.lookup?.()).to.deep.eq({
         wsURL: 'ws://test',
         projectId: 'abc',
         clusterName: `Cluster0-${type}`,
@@ -172,4 +176,133 @@ describe('buildConnectionInfoFromClusterDescription', function () {
         });
     });
   }
+
+  it('should throw if deployment item is missing', function () {
+    try {
+      buildConnectionInfoFromClusterDescription(
+        'ws://test',
+        '123',
+        'abc',
+        {
+          '@provider': 'mock',
+          uniqueId: 'abc',
+          groupId: 'abc',
+          name: 'Cluster0',
+          clusterType: 'REPLICASET',
+          srvAddress: 'test',
+          state: 'test',
+          deploymentItemName: 'test',
+          dataProcessingRegion: { regionalUrl: 'test' },
+        },
+        deployment
+      );
+      expect.fail('Expected method to throw');
+    } catch (err) {
+      expect(err).to.have.property(
+        'message',
+        "Can't build metrics info when deployment item is not found"
+      );
+    }
+  });
+});
+
+describe('AtlasCloudConnectionStorage', function () {
+  const testClusters: Record<
+    string,
+    Partial<ClusterDescriptionWithDataProcessingRegion>
+  > = {
+    Cluster0: {
+      '@provider': 'AWS',
+      groupId: 'abc',
+      name: 'Cluster0',
+      clusterType: 'REPLICASET',
+      srvAddress: 'test',
+      state: 'test',
+      deploymentItemName: 'replicaSet-xxx',
+      dataProcessingRegion: { regionalUrl: 'test' },
+    },
+    NoDeploymentItem: {
+      '@provider': 'AWS',
+      groupId: 'abc',
+      name: 'NoDeploymentItem',
+      clusterType: 'REPLICASET',
+      srvAddress: 'test',
+      state: 'test',
+      deploymentItemName: 'not-found',
+      dataProcessingRegion: { regionalUrl: 'test' },
+    },
+    NoSrvAddress: {
+      '@provider': 'AWS',
+      name: 'NoSrvAddress',
+    },
+    Paused: {
+      '@provider': 'AWS',
+      name: 'Paused',
+      isPaused: true,
+    },
+    WillThrowOnFetch: {
+      '@provider': 'AWS',
+      name: 'WillThrowOnFetch',
+    },
+  };
+
+  describe('#loadAll', function () {
+    it('should load connection descriptions filtering out the ones that failed to fetch', async function () {
+      const atlasService = {
+        cloudEndpoint(path: string) {
+          return path;
+        },
+        driverProxyEndpoint(path: string) {
+          return path;
+        },
+        authenticatedFetch(path: string) {
+          let payload: any;
+          if (path === '/deployment/abc') {
+            payload = deployment;
+          }
+          if (path === '/nds/clusters/abc') {
+            payload = Array.from(Object.values(testClusters));
+          }
+          const { groups } =
+            /^\/nds\/clusters\/abc\/(?<clusterName>.+?)\/.+?$/.exec(path) ?? {
+              groups: undefined,
+            };
+          if (groups?.clusterName) {
+            if (groups?.clusterName === 'WillThrowOnFetch') {
+              return Promise.reject(
+                new Error('Failed to fetch cluster description')
+              );
+            }
+            payload = testClusters[groups.clusterName];
+          }
+          return Promise.resolve({
+            json() {
+              return payload;
+            },
+          });
+        },
+      };
+      const logger = createNoopLogger();
+      const connectionStorage = new AtlasCloudConnectionStorage(
+        atlasService as any,
+        '123',
+        'abc',
+        logger
+      );
+
+      const connectionsPromise = connectionStorage.loadAll();
+
+      expect(connectionsPromise).to.eq(
+        connectionStorage.loadAll(),
+        'Expected loadAll to return the same instance of the loading promise while connections are loading'
+      );
+
+      const connections = await connectionsPromise;
+
+      // We expect all other clusters to be filtered out for one reason or
+      // another
+      expect(connections).to.have.lengthOf(1);
+      expect(connections[0]).to.have.property('id', 'Cluster0');
+    });
+  });
 });

packages/compass-web/src/connection-storage.tsx

@@ -11,6 +11,11 @@ import ConnectionString from 'mongodb-connection-string-url';
 import { createServiceProvider } from 'hadron-app-registry';
 import type { AtlasService } from '@mongodb-js/atlas-service/provider';
 import { atlasServiceLocator } from '@mongodb-js/atlas-service/provider';
+import {
+  mongoLogId,
+  useLogger,
+  type Logger,
+} from '@mongodb-js/compass-logging/provider';
 
 type ElectableSpecs = {
   instanceSize?: string;
@@ -156,7 +161,7 @@ export function buildConnectionInfoFromClusterDescription(
   description: ClusterDescriptionWithDataProcessingRegion,
   deployment: Deployment,
   extraConnectionOptions?: Record<string, any>
-) {
+): ConnectionInfo {
   const connectionString = new ConnectionString(
     `mongodb+srv://${description.srvAddress}`
   );
@@ -221,7 +226,10 @@ export function buildConnectionInfoFromClusterDescription(
   };
 }
 
-class AtlasCloudConnectionStorage
+/**
+ * @internal exported for testing purposes
+ */
+export class AtlasCloudConnectionStorage
   extends InMemoryConnectionStorage
   implements ConnectionStorage
 {
@@ -230,6 +238,7 @@ class AtlasCloudConnectionStorage
     private atlasService: AtlasService,
     private orgId: string,
     private projectId: string,
+    private logger: Logger,
     private extraConnectionOptions?: Record<string, any>
   ) {
     super();
@@ -249,67 +258,95 @@ class AtlasCloudConnectionStorage
           // TODO(CLOUDP-249088): replace with the list request that already
           // contains regional data when it exists instead of fetching
           // one-by-one after the list fetch
-          this.atlasService.cloudEndpoint(`nds/clusters/${this.projectId}`)
+          this.atlasService.cloudEndpoint(`/nds/clusters/${this.projectId}`)
         )
         .then((res) => {
           return res.json() as Promise<ClusterDescription[]>;
         })
         .then((descriptions) => {
           return Promise.all(
-            descriptions
-              .filter((description) => {
-                // Only list fully deployed clusters
-                // TODO(COMPASS-8228): We should probably list all and just
-                // account in the UI for a special state of a deployment as
-                // clusters can become inactive during their runtime and it's
-                // valuable UI info to display
-                return !description.isPaused && !!description.srvAddress;
-              })
-              .map(async (description) => {
-                // Even though nds/clusters will list serverless clusters, to get
-                // the regional description we need to change the url
-                const clusterDescriptionType = isServerless(description)
-                  ? 'serverless'
-                  : 'clusters';
+            descriptions.map(async (description) => {
+              // Even though nds/clusters will list serverless clusters, to get
+              // the regional description we need to change the url
+              const clusterDescriptionType = isServerless(description)
+                ? 'serverless'
+                : 'clusters';
+              try {
                 const res = await this.atlasService.authenticatedFetch(
                   this.atlasService.cloudEndpoint(
-                    `nds/${clusterDescriptionType}/${this.projectId}/${description.name}/regional/clusterDescription`
+                    `/nds/${clusterDescriptionType}/${this.projectId}/${description.name}/regional/clusterDescription`
                   )
                 );
                 return await (res.json() as Promise<ClusterDescriptionWithDataProcessingRegion>);
-              })
+              } catch (err) {
+                this.logger.log.error(
+                  mongoLogId(1_001_000_303),
+                  'LoadAndNormalizeClusterDescriptionInfo',
+                  'Failed to fetch cluster description for cluster',
+                  { clusterName: description.name, error: (err as Error).stack }
+                );
+                return null;
+              }
+            })
           );
         }),
       this.atlasService
         .authenticatedFetch(
-          this.atlasService.cloudEndpoint(`deployment/${this.projectId}`)
+          this.atlasService.cloudEndpoint(`/deployment/${this.projectId}`)
         )
         .then((res) => {
           return res.json() as Promise<Deployment>;
         }),
     ]);
 
-    return clusterDescriptions.map((description) => {
-      return buildConnectionInfoFromClusterDescription(
-        this.atlasService.driverProxyEndpoint(
-          `/clusterConnection/${this.projectId}`
-        ),
-        this.orgId,
-        this.projectId,
-        description,
-        deployment,
-        this.extraConnectionOptions
-      );
-    });
+    return clusterDescriptions
+      .map((description) => {
+        // Clear cases where cluster doesn't have enough metadata
+        //  - Failed to get the description
+        //  - Cluster is paused
+        //  - Cluster is missing an srv address (happens during deployment /
+        //    termination)
+        if (!description || !!description.isPaused || !description.srvAddress) {
+          return null;
+        }
+
+        try {
+          // We will always try to build the metadata, it can fail if deployment
+          // item for the cluster is missing even when description exists
+          // (happens during deployment / termination / weird corner cases of
+          // atlas cluster state)
+          return buildConnectionInfoFromClusterDescription(
+            this.atlasService.driverProxyEndpoint(
+              `/clusterConnection/${this.projectId}`
+            ),
+            this.orgId,
+            this.projectId,
+            description,
+            deployment,
+            this.extraConnectionOptions
+          );
+        } catch (err) {
+          this.logger.log.error(
+            mongoLogId(1_001_000_304),
+            'LoadAndNormalizeClusterDescriptionInfo',
+            'Failed to build connection info from cluster description',
+            { clusterName: description.name, error: (err as Error).stack }
+          );
+
+          return null;
+        }
+      })
+      .filter((connectionInfo): connectionInfo is ConnectionInfo => {
+        return !!connectionInfo;
+      });
   }
 
-  async loadAll(): Promise<ConnectionInfo[]> {
-    try {
-      return (this.loadAllPromise ??=
-        this._loadAndNormalizeClusterDescriptionInfo());
-    } finally {
-      delete this.loadAllPromise;
-    }
+  loadAll(): Promise<ConnectionInfo[]> {
+    this.loadAllPromise ??=
+      this._loadAndNormalizeClusterDescriptionInfo().finally(() => {
+        delete this.loadAllPromise;
+      });
+    return this.loadAllPromise;
   }
 }
 
@@ -358,12 +395,14 @@ export const AtlasCloudConnectionStorageProvider = createServiceProvider(
     const extraConnectionOptions = useContext(
       SandboxExtraConnectionOptionsContext
     );
+    const logger = useLogger('ATLAS-CLOUD-CONNECTION-STORAGE');
     const atlasService = atlasServiceLocator();
     const storage = useRef(
       new AtlasCloudConnectionStorage(
         atlasService,
         orgId,
         projectId,
+        logger,
         extraConnectionOptions
       )
     );