From 12ca3be675952be5d51bbc3697ffe5c6cf5dd122 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Fri, 1 Nov 2024 17:07:41 +0100 Subject: [PATCH 1/5] fix(compass-global-writes): handle loading error COMPASS-8446 --- .../src/components/index.tsx | 5 + .../src/components/states/loading-error.tsx | 26 +++++ .../src/store/reducer.ts | 106 ++++++++++++++---- 3 files changed, 117 insertions(+), 20 deletions(-) create mode 100644 packages/compass-global-writes/src/components/states/loading-error.tsx diff --git a/packages/compass-global-writes/src/components/index.tsx b/packages/compass-global-writes/src/components/index.tsx index 902342e1ce0..d7331022a17 100644 --- a/packages/compass-global-writes/src/components/index.tsx +++ b/packages/compass-global-writes/src/components/index.tsx @@ -16,6 +16,7 @@ import ShardKeyInvalid from './states/shard-key-invalid'; import ShardKeyMismatch from './states/shard-key-mismatch'; import ShardingError from './states/sharding-error'; import IncompleteShardingSetup from './states/incomplete-sharding-setup'; +import LoadingError from './states/loading-error'; const containerStyles = css({ display: 'flex', @@ -90,6 +91,10 @@ function ShardingStateView({ return ; } + if (shardingStatus === ShardingStatuses.LOADING_ERROR) { + return ; + } + return null; } diff --git a/packages/compass-global-writes/src/components/states/loading-error.tsx b/packages/compass-global-writes/src/components/states/loading-error.tsx new file mode 100644 index 00000000000..7dafb76ac64 --- /dev/null +++ b/packages/compass-global-writes/src/components/states/loading-error.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import { ErrorSummary } from '@mongodb-js/compass-components'; +import { connect } from 'react-redux'; +import { type RootState, ShardingStatuses } from '../../store/reducer'; +import { containerStyles } from '../common-styles'; + +interface LoadingErrorProps { + error: string; +} + +export function LoadingError({ error }: LoadingErrorProps) { + return ( +
+ +
+ ); +} + +export default connect((state: RootState) => { + if (state.status !== ShardingStatuses.LOADING_ERROR) { + throw new Error('Error not found in LoadingError'); + } + return { + error: state.error, + }; +})(LoadingError); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 44a655f79d3..c8382f6e9cd 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -46,6 +46,8 @@ enum GlobalWritesActionTypes { UnmanagingNamespaceStarted = 'global-writes/UnmanagingNamespaceStarted', UnmanagingNamespaceFinished = 'global-writes/UnmanagingNamespaceFinished', UnmanagingNamespaceErrored = 'global-writes/UnmanagingNamespaceErrored', + + LoadingFailed = 'global-writes/LoadingFailed', } type ManagedNamespaceFetchedAction = { @@ -53,6 +55,11 @@ type ManagedNamespaceFetchedAction = { managedNamespace?: ManagedNamespace; }; +type LoadingFailedAction = { + type: GlobalWritesActionTypes.LoadingFailed; + error: string; +}; + type NamespaceShardingErrorFetchedAction = { type: GlobalWritesActionTypes.NamespaceShardingErrorFetched; error: string; @@ -125,6 +132,11 @@ export enum ShardingStatuses { */ NOT_READY = 'NOT_READY', + /** + * The status could not be determined because loading failed + */ + LOADING_ERROR = 'LOADING_ERROR', + /** * Namespace is not geo-sharded. */ @@ -209,11 +221,19 @@ export type RootState = { managedNamespace?: ManagedNamespace; shardZones: ShardZoneData[]; } & ( + | { + status: ShardingStatuses.LOADING_ERROR; + shardKey?: ShardKey; + shardingError?: never; + pollingTimeout?: never; + error: string; + } | { status: ShardingStatuses.NOT_READY; shardKey?: never; shardingError?: never; pollingTimeout?: never; + error?: never; } | { status: @@ -225,6 +245,7 @@ export type RootState = { // and then unmanaged shardingError?: never; pollingTimeout?: never; + error?: never; } | { status: ShardingStatuses.SHARDING; @@ -235,6 +256,7 @@ export type RootState = { shardKey?: ShardKey; shardingError?: never; pollingTimeout?: NodeJS.Timeout; + error?: never; } | { status: @@ -244,6 +266,7 @@ export type RootState = { shardKey?: never; shardingError: string; pollingTimeout?: never; + error?: never; } | { status: @@ -257,6 +280,7 @@ export type RootState = { shardKey: ShardKey; shardingError?: never; pollingTimeout?: never; + error?: never; } ); @@ -616,6 +640,25 @@ const reducer: Reducer = (state = initialState, action) => { }; } + if ( + isAction( + action, + GlobalWritesActionTypes.LoadingFailed + ) && + (state.status === ShardingStatuses.NOT_READY || + state.status === ShardingStatuses.SHARDING) + ) { + if (state.pollingTimeout) { + throw new Error('Polling was not stopped'); + } + return { + ...state, + status: ShardingStatuses.LOADING_ERROR, + error: action.error, + pollingTimeout: state.pollingTimeout, + }; + } + return state; }; @@ -644,17 +687,11 @@ export const fetchClusterShardingData = 'Error fetching cluster sharding data', (error as Error).message ); - openToast( - `global-writes-fetch-shard-info-error-${connectionInfoRef.current.id}-${namespace}`, - { - title: `Failed to fetch sharding information: ${ - (error as Error).message - }`, - dismissible: true, - timeout: 5000, - variant: 'important', - } - ); + handleLoadingError({ + error: error as Error, + id: `global-writes-fetch-shard-info-error-${connectionInfoRef.current.id}-${namespace}`, + description: 'Failed to fetch sharding information', + }); } }; @@ -829,6 +866,39 @@ const stopPollingForShardKey = (): GlobalWritesThunkAction< }; }; +const handleLoadingError = ({ + error, + id, + description, +}: { + error: Error; + id: string; + description: string; +}): GlobalWritesThunkAction => { + return (dispatch, getState) => { + const { status } = getState(); + const isPolling = status === ShardingStatuses.SHARDING; + const isInitialLoad = status === ShardingStatuses.NOT_READY; + const errorMessage = `${description} ${error.message}`; + if (isInitialLoad || isPolling) { + if (isPolling) { + dispatch(stopPollingForShardKey()); + } + dispatch({ + type: GlobalWritesActionTypes.LoadingFailed, + error: errorMessage, + }); + return; + } + openToast(id, { + title: errorMessage, + dismissible: true, + timeout: 5000, + variant: 'important', + }); + }; +}; + export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< Promise, NamespaceShardingErrorFetchedAction | NamespaceShardKeyFetchedAction @@ -882,15 +952,11 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< 'Error fetching shard key', (error as Error).message ); - openToast( - `global-writes-fetch-shard-key-error-${connectionInfoRef.current.id}-${namespace}`, - { - title: `Failed to fetch shard key: ${(error as Error).message}`, - dismissible: true, - timeout: 5000, - variant: 'important', - } - ); + handleLoadingError({ + error: error as Error, + id: `global-writes-fetch-shard-key-error-${connectionInfoRef.current.id}-${namespace}`, + description: 'Failed to fetch shard key', + }); } }; }; From b2a6afabd18cc24f1cf04c3fa91f3653198d6909 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 6 Nov 2024 17:34:25 +0100 Subject: [PATCH 2/5] add tests --- .../src/store/index.spec.ts | 94 +++++++++++++++++++ .../src/store/reducer.ts | 26 ++--- 2 files changed, 109 insertions(+), 11 deletions(-) diff --git a/packages/compass-global-writes/src/store/index.spec.ts b/packages/compass-global-writes/src/store/index.spec.ts index 74a1ac056ec..19fb3bf21a6 100644 --- a/packages/compass-global-writes/src/store/index.spec.ts +++ b/packages/compass-global-writes/src/store/index.spec.ts @@ -73,6 +73,9 @@ function createStore({ hasShardKey = () => false, failsOnShardingRequest = () => false, failsOnShardZoneRequest = () => false, + failsToFetchClusterDetails = () => false, + failsToFetchDeploymentStatus = () => false, + failsToFetchShardKey = () => false, authenticatedFetchStub, }: | { @@ -81,6 +84,9 @@ function createStore({ hasShardKey?: () => boolean | AtlasShardKey; failsOnShardingRequest?: () => boolean; failsOnShardZoneRequest?: () => boolean; + failsToFetchClusterDetails?: () => boolean; + failsToFetchDeploymentStatus?: () => boolean; + failsToFetchShardKey?: () => boolean; authenticatedFetchStub?: never; } | { @@ -89,6 +95,9 @@ function createStore({ hasShardKey?: () => boolean | ShardKey; failsOnShardingRequest?: never; failsOnShardZoneRequest?: () => never; + failsToFetchClusterDetails?: never; + failsToFetchDeploymentStatus?: never; + failsToFetchShardKey?: () => boolean; authenticatedFetchStub?: () => void; } = {}): GlobalWritesStore { const atlasService = { @@ -98,6 +107,9 @@ function createStore({ } if (uri.includes('/clusters/')) { + if (failsToFetchClusterDetails()) { + return Promise.reject(new Error('Failed to fetch cluster details')); + } return createAuthFetchResponse({ ...clusterDetails, geoSharding: { @@ -108,6 +120,9 @@ function createStore({ } if (uri.includes('/deploymentStatus/')) { + if (failsToFetchDeploymentStatus()) { + return Promise.reject(new Error('Failed to fetch deployment status')); + } return createAuthFetchResponse({ automationStatus: { processes: hasShardingError() ? [failedShardingProcess] : [], @@ -130,6 +145,10 @@ function createStore({ }), automationAgentAwait: (_meta: unknown, type: string) => { if (type === 'getShardKey') { + if (failsToFetchShardKey()) { + return Promise.reject(new Error('Failed to fetch shardKey')); + } + const shardKey = hasShardKey(); return { response: @@ -188,6 +207,35 @@ describe('GlobalWritesStore Store', function () { }); context('scenarios', function () { + context('initial load fail', function () { + it('fails to fetch cluster details', async function () { + const store = createStore({ + failsToFetchClusterDetails: () => true, + }); + await waitFor(() => { + expect(store.getState().status).to.equal('LOADING_ERROR'); + }); + }); + + it('fails to fetch shard key', async function () { + const store = createStore({ + failsToFetchShardKey: () => true, + }); + await waitFor(() => { + expect(store.getState().status).to.equal('LOADING_ERROR'); + }); + }); + + it('fails to fetch deployment status', async function () { + const store = createStore({ + failsToFetchDeploymentStatus: () => true, + }); + await waitFor(() => { + expect(store.getState().status).to.equal('LOADING_ERROR'); + }); + }); + }); + it('not managed -> sharding -> valid shard key', async function () { let mockShardKey = false; let mockManagedNamespace = false; @@ -291,6 +339,52 @@ describe('GlobalWritesStore Store', function () { }); }); + context('pulling fail', function () { + it('sharding -> error (failed to fetch shard key)', async function () { + let mockFailure = false; + // initial state === sharding + clock = sinon.useFakeTimers({ + shouldAdvanceTime: true, + }); + const store = createStore({ + isNamespaceManaged: () => true, + failsToFetchShardKey: Sinon.fake(() => mockFailure), + }); + await waitFor(() => { + expect(store.getState().status).to.equal('SHARDING'); + }); + + // sharding ends with a request failure + mockFailure = true; + clock.tick(POLLING_INTERVAL); + await waitFor(() => { + expect(store.getState().status).to.equal('LOADING_ERROR'); + }); + }); + + it('sharding -> error (failed to fetch deployment status)', async function () { + let mockFailure = false; + // initial state === sharding + clock = sinon.useFakeTimers({ + shouldAdvanceTime: true, + }); + const store = createStore({ + isNamespaceManaged: () => true, + failsToFetchDeploymentStatus: Sinon.fake(() => mockFailure), + }); + await waitFor(() => { + expect(store.getState().status).to.equal('SHARDING'); + }); + + // sharding ends with a request failure + mockFailure = true; + clock.tick(POLLING_INTERVAL); + await waitFor(() => { + expect(store.getState().status).to.equal('LOADING_ERROR'); + }); + }); + }); + it('sharding -> cancelling request -> not managed', async function () { let mockManagedNamespace = true; confirmationStub.resolves(true); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index c8382f6e9cd..92dc487b38d 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -687,11 +687,13 @@ export const fetchClusterShardingData = 'Error fetching cluster sharding data', (error as Error).message ); - handleLoadingError({ - error: error as Error, - id: `global-writes-fetch-shard-info-error-${connectionInfoRef.current.id}-${namespace}`, - description: 'Failed to fetch sharding information', - }); + dispatch( + handleLoadingError({ + error: error as Error, + id: `global-writes-fetch-shard-info-error-${connectionInfoRef.current.id}-${namespace}`, + description: 'Failed to fetch sharding information', + }) + ); } }; @@ -949,14 +951,16 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< logger.log.error( logger.mongoLogId(1_001_000_333), 'AtlasFetchError', - 'Error fetching shard key', + 'Error fetching shard key / deployment status', (error as Error).message ); - handleLoadingError({ - error: error as Error, - id: `global-writes-fetch-shard-key-error-${connectionInfoRef.current.id}-${namespace}`, - description: 'Failed to fetch shard key', - }); + dispatch( + handleLoadingError({ + error: error as Error, + id: `global-writes-fetch-shard-key-error-${connectionInfoRef.current.id}-${namespace}`, + description: 'Failed to fetch shard key / deployment status', + }) + ); } }; }; From a426adf4322905e34d23a3ec453e2df47d4bd11c Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 7 Nov 2024 13:32:38 +0100 Subject: [PATCH 3/5] fix polling --- packages/compass-global-writes/src/store/reducer.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 92dc487b38d..4cad2f20719 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -881,11 +881,8 @@ const handleLoadingError = ({ const { status } = getState(); const isPolling = status === ShardingStatuses.SHARDING; const isInitialLoad = status === ShardingStatuses.NOT_READY; - const errorMessage = `${description} ${error.message}`; + const errorMessage = `${description}: ${error.message}`; if (isInitialLoad || isPolling) { - if (isPolling) { - dispatch(stopPollingForShardKey()); - } dispatch({ type: GlobalWritesActionTypes.LoadingFailed, error: errorMessage, @@ -903,13 +900,16 @@ const handleLoadingError = ({ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< Promise, - NamespaceShardingErrorFetchedAction | NamespaceShardKeyFetchedAction + | NamespaceShardingErrorFetchedAction + | NamespaceShardKeyFetchedAction + | NextPollingTimeoutClearedAction > => { return async ( dispatch, getState, { atlasGlobalWritesService, logger, connectionInfoRef } ) => { + dispatch({ type: GlobalWritesActionTypes.NextPollingTimeoutCleared }); const { namespace, status, managedNamespace } = getState(); try { @@ -958,7 +958,7 @@ export const fetchNamespaceShardKey = (): GlobalWritesThunkAction< handleLoadingError({ error: error as Error, id: `global-writes-fetch-shard-key-error-${connectionInfoRef.current.id}-${namespace}`, - description: 'Failed to fetch shard key / deployment status', + description: 'Failed to fetch shard key or deployment status', }) ); } From deab82004a993b5ad112a8e7b8fdbbbc84e095cd Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 7 Nov 2024 13:36:29 +0100 Subject: [PATCH 4/5] add test --- .../components/states/loading-error.spec.tsx | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 packages/compass-global-writes/src/components/states/loading-error.spec.tsx diff --git a/packages/compass-global-writes/src/components/states/loading-error.spec.tsx b/packages/compass-global-writes/src/components/states/loading-error.spec.tsx new file mode 100644 index 00000000000..263b2b4fdc4 --- /dev/null +++ b/packages/compass-global-writes/src/components/states/loading-error.spec.tsx @@ -0,0 +1,21 @@ +import React from 'react'; +import { expect } from 'chai'; +import { screen, userEvent } from '@mongodb-js/testing-library-compass'; +import { LoadingError } from './loading-error'; +import { renderWithStore } from '../../../tests/create-store'; +import Sinon from 'sinon'; + +const error = 'Test failure'; + +function renderWithProps( + props?: Partial> +) { + return renderWithStore(); +} + +describe('LoadingError', function () { + it('renders the error', async function () { + await renderWithProps(); + expect(screen.getByText(error)).to.exist; + }); +}); From 9ef0db65092a46ceae36905020593d2d80b6b1df Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 7 Nov 2024 13:42:14 +0100 Subject: [PATCH 5/5] adjustments --- .../src/components/states/loading-error.spec.tsx | 3 +-- .../src/components/states/loading-error.tsx | 2 +- .../compass-global-writes/src/store/reducer.ts | 14 +++++++------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/compass-global-writes/src/components/states/loading-error.spec.tsx b/packages/compass-global-writes/src/components/states/loading-error.spec.tsx index 263b2b4fdc4..2ed220dd9bd 100644 --- a/packages/compass-global-writes/src/components/states/loading-error.spec.tsx +++ b/packages/compass-global-writes/src/components/states/loading-error.spec.tsx @@ -1,9 +1,8 @@ import React from 'react'; import { expect } from 'chai'; -import { screen, userEvent } from '@mongodb-js/testing-library-compass'; +import { screen } from '@mongodb-js/testing-library-compass'; import { LoadingError } from './loading-error'; import { renderWithStore } from '../../../tests/create-store'; -import Sinon from 'sinon'; const error = 'Test failure'; diff --git a/packages/compass-global-writes/src/components/states/loading-error.tsx b/packages/compass-global-writes/src/components/states/loading-error.tsx index 7dafb76ac64..8a508c38316 100644 --- a/packages/compass-global-writes/src/components/states/loading-error.tsx +++ b/packages/compass-global-writes/src/components/states/loading-error.tsx @@ -21,6 +21,6 @@ export default connect((state: RootState) => { throw new Error('Error not found in LoadingError'); } return { - error: state.error, + error: state.loadingError, }; })(LoadingError); diff --git a/packages/compass-global-writes/src/store/reducer.ts b/packages/compass-global-writes/src/store/reducer.ts index 4cad2f20719..aa6904b0eee 100644 --- a/packages/compass-global-writes/src/store/reducer.ts +++ b/packages/compass-global-writes/src/store/reducer.ts @@ -226,14 +226,14 @@ export type RootState = { shardKey?: ShardKey; shardingError?: never; pollingTimeout?: never; - error: string; + loadingError: string; } | { status: ShardingStatuses.NOT_READY; shardKey?: never; shardingError?: never; pollingTimeout?: never; - error?: never; + loadingError?: never; } | { status: @@ -245,7 +245,7 @@ export type RootState = { // and then unmanaged shardingError?: never; pollingTimeout?: never; - error?: never; + loadingError?: never; } | { status: ShardingStatuses.SHARDING; @@ -256,7 +256,7 @@ export type RootState = { shardKey?: ShardKey; shardingError?: never; pollingTimeout?: NodeJS.Timeout; - error?: never; + loadingError?: never; } | { status: @@ -266,7 +266,7 @@ export type RootState = { shardKey?: never; shardingError: string; pollingTimeout?: never; - error?: never; + loadingError?: never; } | { status: @@ -280,7 +280,7 @@ export type RootState = { shardKey: ShardKey; shardingError?: never; pollingTimeout?: never; - error?: never; + loadingError?: never; } ); @@ -654,7 +654,7 @@ const reducer: Reducer = (state = initialState, action) => { return { ...state, status: ShardingStatuses.LOADING_ERROR, - error: action.error, + loadingError: action.error, pollingTimeout: state.pollingTimeout, }; }