Skip to content

Commit 1aa74b7

Browse files
committed
fixup: remove atlas restrictive check, add unit tests
1 parent e9da815 commit 1aa74b7

File tree

2 files changed

+140
-27
lines changed

2 files changed

+140
-27
lines changed

packages/compass-connections/src/stores/connections-store-redux.spec.tsx

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {
1010
} from '@mongodb-js/testing-library-compass';
1111
import React from 'react';
1212
import { InMemoryConnectionStorage } from '@mongodb-js/connection-storage/provider';
13+
import { getDataServiceForConnection } from './connections-store-redux';
14+
import { type ConnectionInfo } from '@mongodb-js/connection-info';
1315

1416
const mockConnections = [
1517
{
@@ -34,6 +36,13 @@ const mockConnections = [
3436
},
3537
];
3638

39+
const connectionInfoWithAtlasMetadata = {
40+
...createDefaultConnectionInfo(),
41+
atlasMetadata: {
42+
clusterName: 'pineapple',
43+
} as ConnectionInfo['atlasMetadata'],
44+
};
45+
3746
function renderCompassConnections(opts?: RenderConnectionsOptions) {
3847
return render(
3948
<div>
@@ -274,6 +283,101 @@ describe('CompassConnections store', function () {
274283
await connectionStorage.load({ id: mockConnections[0].id })
275284
).to.have.nested.property('favorite.name', 'turtles');
276285
});
286+
287+
it('should ignore server heartbeat failed events that are not non-retryable error codes', async function () {
288+
const { connectionsStore } = renderCompassConnections({
289+
connectFn: async () => {
290+
await wait(1);
291+
return {};
292+
},
293+
});
294+
295+
// Wait till we're connected.
296+
await connectionsStore.actions.connect(connectionInfoWithAtlasMetadata);
297+
298+
const connections = connectionsStore.getState().connections;
299+
expect(connections.ids).to.have.lengthOf(1);
300+
301+
const dataService = getDataServiceForConnection(
302+
connectionInfoWithAtlasMetadata.id
303+
);
304+
305+
let didDisconnect = false;
306+
let didCheckForConnected = false;
307+
sinon.stub(dataService, 'disconnect').callsFake(async () => {
308+
didDisconnect = true;
309+
return Promise.resolve();
310+
});
311+
dataService.isConnected = () => {
312+
// If this is called we know the error wasn't handled properly.
313+
didCheckForConnected = true;
314+
return true;
315+
};
316+
317+
let didReceiveCallToHeartbeatFailedListener = false;
318+
dataService.on('serverHeartbeatFailed', () => {
319+
didReceiveCallToHeartbeatFailedListener = true;
320+
});
321+
322+
// Send a heartbeat fail with an error that's not a non-retryable error code.
323+
dataService['emit']('serverHeartbeatFailed', {
324+
failure: new Error('code: 1234, Not the error we are looking for'),
325+
});
326+
327+
// Wait for the listener to handle the message.
328+
await waitFor(() => {
329+
expect(didReceiveCallToHeartbeatFailedListener).to.be.true;
330+
});
331+
await wait(1);
332+
333+
expect(didDisconnect).to.be.false;
334+
expect(didCheckForConnected).to.be.false;
335+
});
336+
337+
it('should listen for non-retryable errors on server heartbeat failed events and disconnect the data service when encountered', async function () {
338+
const { connectionsStore } = renderCompassConnections({
339+
connectFn: async () => {
340+
await wait(1);
341+
return {};
342+
},
343+
});
344+
345+
// Wait till we're connected.
346+
await connectionsStore.actions.connect(connectionInfoWithAtlasMetadata);
347+
348+
const connections = connectionsStore.getState().connections;
349+
expect(connections.ids).to.have.lengthOf(1);
350+
351+
const dataService = getDataServiceForConnection(
352+
connectionInfoWithAtlasMetadata.id
353+
);
354+
355+
let didDisconnect = false;
356+
sinon.stub(dataService, 'disconnect').callsFake(async () => {
357+
didDisconnect = true;
358+
return Promise.resolve();
359+
});
360+
dataService.isConnected = () => true;
361+
362+
// Send a heartbeat fail with an error that's a non-retryable error code.
363+
dataService['emit']('serverHeartbeatFailed', {
364+
failure: new Error('code: 3003, reason: Insufficient permissions'),
365+
});
366+
367+
await waitFor(() => {
368+
expect(didDisconnect).to.be.true;
369+
});
370+
371+
await waitFor(function () {
372+
const titleNode = screen.getByText('Unable to connect to pineapple');
373+
expect(titleNode).to.be.visible;
374+
375+
const descriptionNode = screen.getByText(
376+
'Reason: Insufficient permissions. To use continue to use this connection either disconnect and reconnect, or refresh your page.'
377+
);
378+
expect(descriptionNode).to.be.visible;
379+
});
380+
});
277381
});
278382

279383
describe('#saveAndConnect', function () {

packages/compass-connections/src/stores/connections-store-redux.ts

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,6 +1466,15 @@ function isAtlasStreamsInstance(
14661466
}
14671467
}
14681468

1469+
// We listen for non-retry-able errors on failed server heartbeats.
1470+
// These can happen when:
1471+
// - A user's session has ended.
1472+
// - The user's roles have changed.
1473+
// - The cluster / group they are trying to connect to has since been deleted.
1474+
// When we encounter one we disconnect. This is to avoid polluting logs/metrics
1475+
// and to avoid constantly retrying to connect when we know it'll fail.
1476+
// These error codes can be found at
1477+
// https://github.com/10gen/mms/blob/de2a9c463cfe530efb8e2a0941033e8207b6cb11/server/src/main/com/xgen/cloud/services/clusterconnection/runtime/res/CustomCloseCodes.java
14691478
const NonRetryableErrorCodes = [3000, 3003, 4004, 1008] as const;
14701479
const NonRetryableErrorDescriptionFallbacks: {
14711480
[code in typeof NonRetryableErrorCodes[number]]: string;
@@ -1501,7 +1510,9 @@ const openConnectionClosedWithNonRetryableErrorToast = (
15011510
) => {
15021511
openToast(`non-retryable-error-encountered--${connectionInfo.id}`, {
15031512
title: `Unable to connect to ${getConnectionTitle(connectionInfo)}`,
1504-
description: `Reason: ${getDescriptionForNonRetryableError(error)}`,
1513+
description: `Reason: ${getDescriptionForNonRetryableError(
1514+
error
1515+
)}. To use continue to use this connection either disconnect and reconnect, or refresh your page.`,
15051516
variant: 'warning',
15061517
});
15071518
};
@@ -1701,34 +1712,32 @@ const connectWithOptions = (
17011712
}
17021713

17031714
let showedNonRetryableErrorToast = false;
1704-
if (connectionInfo.atlasMetadata) {
1705-
// When we're on cloud we listen for non-retry-able errors on failed server
1706-
// heartbeats. These can happen when:
1707-
// - A user's session has ended.
1708-
// - The user's roles have changed.
1709-
// - The cluster / group they are trying to connect to has since been deleted.
1710-
// When we encounter one we disconnect. This is to avoid polluting logs/metrics
1711-
// and to avoid constantly retrying to connect when we know it'll fail.
1712-
dataService.on(
1713-
'serverHeartbeatFailed',
1714-
(evt: ServerHeartbeatFailedEvent) => {
1715-
if (!isNonRetryableHeartbeatFailure(evt)) {
1716-
return;
1717-
}
1718-
1719-
if (!dataService.isConnected() || showedNonRetryableErrorToast) {
1720-
return;
1721-
}
1715+
// Listen for non-retry-able errors on failed server heartbeats.
1716+
// These can happen when:
1717+
// - A user's session has ended.
1718+
// - The user's roles have changed.
1719+
// - The cluster / group they are trying to connect to has since been deleted.
1720+
// When we encounter one we disconnect. This is to avoid polluting logs/metrics
1721+
// and to avoid constantly retrying to connect when we know it'll fail.
1722+
dataService.on(
1723+
'serverHeartbeatFailed',
1724+
(evt: ServerHeartbeatFailedEvent) => {
1725+
if (!isNonRetryableHeartbeatFailure(evt)) {
1726+
return;
1727+
}
17221728

1723-
openConnectionClosedWithNonRetryableErrorToast(
1724-
connectionInfo,
1725-
evt.failure
1726-
);
1727-
showedNonRetryableErrorToast = true;
1728-
void dataService.disconnect();
1729+
if (!dataService.isConnected() || showedNonRetryableErrorToast) {
1730+
return;
17291731
}
1730-
);
1731-
}
1732+
1733+
openConnectionClosedWithNonRetryableErrorToast(
1734+
connectionInfo,
1735+
evt.failure
1736+
);
1737+
showedNonRetryableErrorToast = true;
1738+
void dataService.disconnect();
1739+
}
1740+
);
17321741

17331742
dataService.on('oidcAuthFailed', (error) => {
17341743
openToast('oidc-auth-failed', {

0 commit comments

Comments
 (0)