Skip to content

Commit e20763a

Browse files
gribnoysupaddaleax
andauthored
chore(web): catch and log errors when building connection info instead of throwing COMPASS-8848 (#6635)
* chore(web): catch and log errors when building connection info instead of throwing * chore(web): stricter regex in test Co-authored-by: Anna Henningsen <[email protected]> --------- Co-authored-by: Anna Henningsen <[email protected]>
1 parent d44cac0 commit e20763a

File tree

2 files changed

+214
-42
lines changed

2 files changed

+214
-42
lines changed

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

Lines changed: 135 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { expect } from 'chai';
2-
import { buildConnectionInfoFromClusterDescription } from './connection-storage';
2+
import { createNoopLogger } from '@mongodb-js/compass-logging/provider';
3+
import {
4+
buildConnectionInfoFromClusterDescription,
5+
AtlasCloudConnectionStorage,
6+
} from './connection-storage';
37
import type { ClusterDescriptionWithDataProcessingRegion } from './connection-storage';
48

59
const deployment = {
@@ -140,7 +144,7 @@ describe('buildConnectionInfoFromClusterDescription', function () {
140144
connectionString
141145
);
142146

143-
expect(connectionInfo.connectionOptions.lookup()).to.deep.eq({
147+
expect(connectionInfo.connectionOptions.lookup?.()).to.deep.eq({
144148
wsURL: 'ws://test',
145149
projectId: 'abc',
146150
clusterName: `Cluster0-${type}`,
@@ -172,4 +176,133 @@ describe('buildConnectionInfoFromClusterDescription', function () {
172176
});
173177
});
174178
}
179+
180+
it('should throw if deployment item is missing', function () {
181+
try {
182+
buildConnectionInfoFromClusterDescription(
183+
'ws://test',
184+
'123',
185+
'abc',
186+
{
187+
'@provider': 'mock',
188+
uniqueId: 'abc',
189+
groupId: 'abc',
190+
name: 'Cluster0',
191+
clusterType: 'REPLICASET',
192+
srvAddress: 'test',
193+
state: 'test',
194+
deploymentItemName: 'test',
195+
dataProcessingRegion: { regionalUrl: 'test' },
196+
},
197+
deployment
198+
);
199+
expect.fail('Expected method to throw');
200+
} catch (err) {
201+
expect(err).to.have.property(
202+
'message',
203+
"Can't build metrics info when deployment item is not found"
204+
);
205+
}
206+
});
207+
});
208+
209+
describe('AtlasCloudConnectionStorage', function () {
210+
const testClusters: Record<
211+
string,
212+
Partial<ClusterDescriptionWithDataProcessingRegion>
213+
> = {
214+
Cluster0: {
215+
'@provider': 'AWS',
216+
groupId: 'abc',
217+
name: 'Cluster0',
218+
clusterType: 'REPLICASET',
219+
srvAddress: 'test',
220+
state: 'test',
221+
deploymentItemName: 'replicaSet-xxx',
222+
dataProcessingRegion: { regionalUrl: 'test' },
223+
},
224+
NoDeploymentItem: {
225+
'@provider': 'AWS',
226+
groupId: 'abc',
227+
name: 'NoDeploymentItem',
228+
clusterType: 'REPLICASET',
229+
srvAddress: 'test',
230+
state: 'test',
231+
deploymentItemName: 'not-found',
232+
dataProcessingRegion: { regionalUrl: 'test' },
233+
},
234+
NoSrvAddress: {
235+
'@provider': 'AWS',
236+
name: 'NoSrvAddress',
237+
},
238+
Paused: {
239+
'@provider': 'AWS',
240+
name: 'Paused',
241+
isPaused: true,
242+
},
243+
WillThrowOnFetch: {
244+
'@provider': 'AWS',
245+
name: 'WillThrowOnFetch',
246+
},
247+
};
248+
249+
describe('#loadAll', function () {
250+
it('should load connection descriptions filtering out the ones that failed to fetch', async function () {
251+
const atlasService = {
252+
cloudEndpoint(path: string) {
253+
return path;
254+
},
255+
driverProxyEndpoint(path: string) {
256+
return path;
257+
},
258+
authenticatedFetch(path: string) {
259+
let payload: any;
260+
if (path === '/deployment/abc') {
261+
payload = deployment;
262+
}
263+
if (path === '/nds/clusters/abc') {
264+
payload = Array.from(Object.values(testClusters));
265+
}
266+
const { groups } =
267+
/^\/nds\/clusters\/abc\/(?<clusterName>.+?)\/.+?$/.exec(path) ?? {
268+
groups: undefined,
269+
};
270+
if (groups?.clusterName) {
271+
if (groups?.clusterName === 'WillThrowOnFetch') {
272+
return Promise.reject(
273+
new Error('Failed to fetch cluster description')
274+
);
275+
}
276+
payload = testClusters[groups.clusterName];
277+
}
278+
return Promise.resolve({
279+
json() {
280+
return payload;
281+
},
282+
});
283+
},
284+
};
285+
const logger = createNoopLogger();
286+
const connectionStorage = new AtlasCloudConnectionStorage(
287+
atlasService as any,
288+
'123',
289+
'abc',
290+
logger
291+
);
292+
293+
const connectionsPromise = connectionStorage.loadAll();
294+
295+
expect(connectionsPromise).to.eq(
296+
connectionStorage.loadAll(),
297+
'Expected loadAll to return the same instance of the loading promise while connections are loading'
298+
);
299+
300+
const connections = await connectionsPromise;
301+
302+
// We expect all other clusters to be filtered out for one reason or
303+
// another
304+
expect(connections).to.have.lengthOf(1);
305+
expect(connections[0]).to.have.property('id', 'Cluster0');
306+
});
307+
});
175308
});

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

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import ConnectionString from 'mongodb-connection-string-url';
1111
import { createServiceProvider } from 'hadron-app-registry';
1212
import type { AtlasService } from '@mongodb-js/atlas-service/provider';
1313
import { atlasServiceLocator } from '@mongodb-js/atlas-service/provider';
14+
import {
15+
mongoLogId,
16+
useLogger,
17+
type Logger,
18+
} from '@mongodb-js/compass-logging/provider';
1419

1520
type ElectableSpecs = {
1621
instanceSize?: string;
@@ -156,7 +161,7 @@ export function buildConnectionInfoFromClusterDescription(
156161
description: ClusterDescriptionWithDataProcessingRegion,
157162
deployment: Deployment,
158163
extraConnectionOptions?: Record<string, any>
159-
) {
164+
): ConnectionInfo {
160165
const connectionString = new ConnectionString(
161166
`mongodb+srv://${description.srvAddress}`
162167
);
@@ -221,7 +226,10 @@ export function buildConnectionInfoFromClusterDescription(
221226
};
222227
}
223228

224-
class AtlasCloudConnectionStorage
229+
/**
230+
* @internal exported for testing purposes
231+
*/
232+
export class AtlasCloudConnectionStorage
225233
extends InMemoryConnectionStorage
226234
implements ConnectionStorage
227235
{
@@ -230,6 +238,7 @@ class AtlasCloudConnectionStorage
230238
private atlasService: AtlasService,
231239
private orgId: string,
232240
private projectId: string,
241+
private logger: Logger,
233242
private extraConnectionOptions?: Record<string, any>
234243
) {
235244
super();
@@ -249,67 +258,95 @@ class AtlasCloudConnectionStorage
249258
// TODO(CLOUDP-249088): replace with the list request that already
250259
// contains regional data when it exists instead of fetching
251260
// one-by-one after the list fetch
252-
this.atlasService.cloudEndpoint(`nds/clusters/${this.projectId}`)
261+
this.atlasService.cloudEndpoint(`/nds/clusters/${this.projectId}`)
253262
)
254263
.then((res) => {
255264
return res.json() as Promise<ClusterDescription[]>;
256265
})
257266
.then((descriptions) => {
258267
return Promise.all(
259-
descriptions
260-
.filter((description) => {
261-
// Only list fully deployed clusters
262-
// TODO(COMPASS-8228): We should probably list all and just
263-
// account in the UI for a special state of a deployment as
264-
// clusters can become inactive during their runtime and it's
265-
// valuable UI info to display
266-
return !description.isPaused && !!description.srvAddress;
267-
})
268-
.map(async (description) => {
269-
// Even though nds/clusters will list serverless clusters, to get
270-
// the regional description we need to change the url
271-
const clusterDescriptionType = isServerless(description)
272-
? 'serverless'
273-
: 'clusters';
268+
descriptions.map(async (description) => {
269+
// Even though nds/clusters will list serverless clusters, to get
270+
// the regional description we need to change the url
271+
const clusterDescriptionType = isServerless(description)
272+
? 'serverless'
273+
: 'clusters';
274+
try {
274275
const res = await this.atlasService.authenticatedFetch(
275276
this.atlasService.cloudEndpoint(
276-
`nds/${clusterDescriptionType}/${this.projectId}/${description.name}/regional/clusterDescription`
277+
`/nds/${clusterDescriptionType}/${this.projectId}/${description.name}/regional/clusterDescription`
277278
)
278279
);
279280
return await (res.json() as Promise<ClusterDescriptionWithDataProcessingRegion>);
280-
})
281+
} catch (err) {
282+
this.logger.log.error(
283+
mongoLogId(1_001_000_303),
284+
'LoadAndNormalizeClusterDescriptionInfo',
285+
'Failed to fetch cluster description for cluster',
286+
{ clusterName: description.name, error: (err as Error).stack }
287+
);
288+
return null;
289+
}
290+
})
281291
);
282292
}),
283293
this.atlasService
284294
.authenticatedFetch(
285-
this.atlasService.cloudEndpoint(`deployment/${this.projectId}`)
295+
this.atlasService.cloudEndpoint(`/deployment/${this.projectId}`)
286296
)
287297
.then((res) => {
288298
return res.json() as Promise<Deployment>;
289299
}),
290300
]);
291301

292-
return clusterDescriptions.map((description) => {
293-
return buildConnectionInfoFromClusterDescription(
294-
this.atlasService.driverProxyEndpoint(
295-
`/clusterConnection/${this.projectId}`
296-
),
297-
this.orgId,
298-
this.projectId,
299-
description,
300-
deployment,
301-
this.extraConnectionOptions
302-
);
303-
});
302+
return clusterDescriptions
303+
.map((description) => {
304+
// Clear cases where cluster doesn't have enough metadata
305+
// - Failed to get the description
306+
// - Cluster is paused
307+
// - Cluster is missing an srv address (happens during deployment /
308+
// termination)
309+
if (!description || !!description.isPaused || !description.srvAddress) {
310+
return null;
311+
}
312+
313+
try {
314+
// We will always try to build the metadata, it can fail if deployment
315+
// item for the cluster is missing even when description exists
316+
// (happens during deployment / termination / weird corner cases of
317+
// atlas cluster state)
318+
return buildConnectionInfoFromClusterDescription(
319+
this.atlasService.driverProxyEndpoint(
320+
`/clusterConnection/${this.projectId}`
321+
),
322+
this.orgId,
323+
this.projectId,
324+
description,
325+
deployment,
326+
this.extraConnectionOptions
327+
);
328+
} catch (err) {
329+
this.logger.log.error(
330+
mongoLogId(1_001_000_304),
331+
'LoadAndNormalizeClusterDescriptionInfo',
332+
'Failed to build connection info from cluster description',
333+
{ clusterName: description.name, error: (err as Error).stack }
334+
);
335+
336+
return null;
337+
}
338+
})
339+
.filter((connectionInfo): connectionInfo is ConnectionInfo => {
340+
return !!connectionInfo;
341+
});
304342
}
305343

306-
async loadAll(): Promise<ConnectionInfo[]> {
307-
try {
308-
return (this.loadAllPromise ??=
309-
this._loadAndNormalizeClusterDescriptionInfo());
310-
} finally {
311-
delete this.loadAllPromise;
312-
}
344+
loadAll(): Promise<ConnectionInfo[]> {
345+
this.loadAllPromise ??=
346+
this._loadAndNormalizeClusterDescriptionInfo().finally(() => {
347+
delete this.loadAllPromise;
348+
});
349+
return this.loadAllPromise;
313350
}
314351
}
315352

@@ -358,12 +395,14 @@ export const AtlasCloudConnectionStorageProvider = createServiceProvider(
358395
const extraConnectionOptions = useContext(
359396
SandboxExtraConnectionOptionsContext
360397
);
398+
const logger = useLogger('ATLAS-CLOUD-CONNECTION-STORAGE');
361399
const atlasService = atlasServiceLocator();
362400
const storage = useRef(
363401
new AtlasCloudConnectionStorage(
364402
atlasService,
365403
orgId,
366404
projectId,
405+
logger,
367406
extraConnectionOptions
368407
)
369408
);

0 commit comments

Comments
 (0)