diff --git a/packages/atlas-service/src/components/atlas-connection-status.tsx b/packages/atlas-service/src/components/atlas-connection-status.tsx index dcc0d1bf976..f803089e7fc 100644 --- a/packages/atlas-service/src/components/atlas-connection-status.tsx +++ b/packages/atlas-service/src/components/atlas-connection-status.tsx @@ -11,7 +11,7 @@ import { spacing, useDarkMode, } from '@mongodb-js/compass-components'; -import { useAtlasLoginActions, useAtlasSignedInUser } from '../provider'; +import { useAtlasLoginActions, useAtlasSignInStatus } from '../provider'; const containerStyles = css({ display: 'flex', @@ -57,7 +57,7 @@ export const AtlasConnectionStatus: React.FunctionComponent< AtlasConnectionStatusProps > = ({ 'data-testid': dataTestId = 'atlas-connection-status' }) => { const darkMode = useDarkMode(); - const userInfo = useAtlasSignedInUser(); + const signInStatus = useAtlasSignInStatus(); const { signOut } = useAtlasLoginActions(); const handleDisconnect = useCallback(() => { @@ -76,7 +76,7 @@ export const AtlasConnectionStatus: React.FunctionComponent< })(); }, [signOut]); - if (!userInfo) { + if (!signInStatus.user) { return null; } diff --git a/packages/atlas-service/src/provider.tsx b/packages/atlas-service/src/provider.tsx index 7493abc2746..72b83a50b25 100644 --- a/packages/atlas-service/src/provider.tsx +++ b/packages/atlas-service/src/provider.tsx @@ -68,9 +68,8 @@ export { AtlasAuthService } from './atlas-auth-service'; export type { AtlasService } from './atlas-service'; export type { AtlasUserInfo } from './renderer'; export { - useAtlasSignedInUser, + useAtlasSignInStatus, useAtlasLoginActions, - useIsAtlasSignInStateResolved, } from './store/atlas-signin-store-context'; export type { AtlasLoginActions } from './store/atlas-signin-store-context'; export { AtlasConnectionStatus } from './components/atlas-connection-status'; diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 3f9ea85b969..ed3aeba3652 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -7,6 +7,7 @@ import { AttemptStateMap, performSignInAttempt, signOut, + SIGN_IN_TIMEOUT_MS, } from './atlas-signin-reducer'; import { expect } from 'chai'; import { configureStore } from './atlas-signin-store'; @@ -84,7 +85,7 @@ describe('atlasSignInReducer', function () { const restorePromise = store.dispatch(restoreSignInState()); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(store.getState()).to.have.nested.property('state', 'restoring'); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledTwice; expect(store.getState()).to.have.nested.property('state', 'success'); // Intentionally returning false here so that if action would affect @@ -106,7 +107,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(mockAtlasService.signIn).not.to.have.been.called; expect(store.getState()).to.have.nested.property('state', 'success'); @@ -122,7 +123,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(mockAtlasService.signIn).to.have.been.calledOnce; expect(store.getState()).to.have.nested.property('state', 'success'); @@ -137,7 +138,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - const signInPromise = store.dispatch(signIn()); + const signInPromise = store.dispatch(signIn({ entrypoint: 'unknown' })); // Avoid unhandled rejections AttemptStateMap.get(attemptId)?.promise.catch(() => {}); await signInPromise; @@ -170,24 +171,116 @@ describe('atlasSignInReducer', function () { const mockAtlasService = { isAuthenticated: isAuthenticatedStub, }; + const track = sandbox.stub(); const store = configureStore({ atlasAuthService: mockAtlasService as any, + track, }); - void store.dispatch(performSignInAttempt()).catch(() => {}); + void store.dispatch(performSignInAttempt()); // Give it some time for start the sign in attempt. It will be waiting // at isAuthenticated, which never resolves. await new Promise((resolve) => setTimeout(resolve, 100)); store.dispatch(cancelSignIn()); expect(store.getState()).to.have.nested.property('state', 'canceled'); + expect(store.getState()).to.have.property('attemptNumber', 2); expect(isAuthenticatedStub).to.have.been.calledOnce; + expect(track).to.have.been.calledWith('Atlas Sign In Canceled', {}); + }); + + it('should not track a cancel when no sign in is in progress', function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: {} as any, + track, + }); + + store.dispatch(cancelSignIn()); + + expect(track).to.not.have.been.calledWith('Atlas Sign In Canceled'); + }); + }); + + describe('sign in timeout', function () { + let clock: Sinon.SinonFakeTimers; + const ENTRYPOINT = 'assistant-tool-atlas-connection-error-debugger'; + + beforeEach(function () { + clock = sandbox.useFakeTimers({ shouldAdvanceTime: true }); + }); + + afterEach(function () { + clock.restore(); + sandbox.restore(); + }); + + async function driveToTimeout() { + const isAuthenticatedStub = sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => { + reject(signal.reason); + }); + }); + }); + const track = sandbox.stub(); + const openToast = sandbox.stub(); + sandbox.replaceGetter(compassComponents, 'openToast', () => openToast); + const store = configureStore({ + atlasAuthService: { isAuthenticated: isAuthenticatedStub } as any, + track, + }); + + const attemptPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + + await clock.tickAsync(0); + expect(store.getState()).to.have.nested.property('state', 'in-progress'); + + // Advance past the timeout. + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + const result = await attemptPromise; + + return { store, track, openToast, result }; + } + + it('resets the state to timed out', async function () { + const { store, result } = await driveToTimeout(); + + expect(store.getState()).to.have.nested.property('state', 'timed-out'); + expect(store.getState()).to.have.property('attemptNumber', 2); + expect(store.getState()).to.have.nested.property( + 'currentAttemptId', + null + ); + expect(result).to.deep.equal({ status: 'timed-out' }); + }); + + it('tracks the timed out event with the entrypoint', async function () { + const { track } = await driveToTimeout(); + + expect( + track.withArgs('Atlas Sign In Timed Out', { entrypoint: ENTRYPOINT }) + ).to.have.been.calledOnce; + }); + + it('shows a toast informing the user that sign in timed out', async function () { + const { openToast } = await driveToTimeout(); + + expect(openToast.withArgs('atlas-timed-out')).to.have.been.calledOnce; + expect(openToast.lastCall.args[1]).to.include({ + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + }); }); }); describe('performSignInAttempt', function () { - it('should resolve when sign in flow finishes', async function () { + it('should resolve with a success result when sign in flow finishes', async function () { const mockAtlasService = { isAuthenticated: sandbox.stub().resolves(false), signIn: sandbox.stub().resolves({ sub: '1234' }), @@ -196,11 +289,15 @@ describe('atlasSignInReducer', function () { const store = configureStore({ atlasAuthService: mockAtlasService as any, }); - await store.dispatch(performSignInAttempt()); + const result = await store.dispatch(performSignInAttempt()); + expect(result).to.deep.equal({ + status: 'success', + userInfo: { sub: '1234' }, + }); expect(store.getState()).to.have.property('state', 'success'); }); - it('should reject if sign in fails', async function () { + it('should resolve with an error result if sign in fails', async function () { const mockAtlasService = { isAuthenticated: sandbox.stub().resolves(false), signIn: sandbox.stub().rejects(new Error('Sign in failed')), @@ -209,16 +306,14 @@ describe('atlasSignInReducer', function () { const store = configureStore({ atlasAuthService: mockAtlasService as any, }); - try { - await store.dispatch(performSignInAttempt()); - expect.fail('Expected performSignInAttempt action to throw'); - } catch (err) { - expect(err).to.have.property('message', 'Sign in failed'); - } + const result = await store.dispatch(performSignInAttempt()); + expect(result).to.have.property('status', 'error'); + expect(result).to.have.nested.property('error.message', 'Sign in failed'); expect(store.getState()).to.have.property('state', 'error'); + expect(store.getState()).to.have.property('attemptNumber', 2); }); - it('should reject if provided signal was aborted', async function () { + it('should resolve with a canceled result if provided signal was aborted', async function () { let resolveSignInCalled = () => {}; const signInCalled: Promise = new Promise( (resolve) => (resolveSignInCalled = resolve) @@ -238,13 +333,12 @@ describe('atlasSignInReducer', function () { const signInPromise = store.dispatch( performSignInAttempt({ signal: c.signal }) ); - c.abort(new Error('Aborted from outside')); - try { - await signInPromise; - throw new Error('Expected signInPromise to throw'); - } catch (err) { - expect(err).to.have.property('message', 'Aborted from outside'); - } + const err = new Error('Aborted from outside'); + c.abort(err); + expect(await signInPromise).to.deep.equal({ + status: 'canceled', + reason: err, + }); expect(store.getState()).to.have.property('state', 'canceled'); // Ensure that we are not leaving a dangling store operation that would conflict with our mocks being reset. @@ -264,14 +358,18 @@ describe('atlasSignInReducer', function () { const firstAttemptPromise = store.dispatch(performSignInAttempt()); const secondAttemptPromise = store.dispatch(performSignInAttempt()); - const [firstUserInfo, secondUserInfo] = await Promise.all([ + const [firstResult, secondResult] = await Promise.all([ firstAttemptPromise, secondAttemptPromise, ]); - // the second call should not have triggered a second signIn call, and both should have the same userInfo + // the second call should not have triggered a second signIn call, and both should resolve to the same result expect(mockAtlasService.signIn).to.have.been.calledOnce; - expect(firstUserInfo).to.deep.equal(secondUserInfo); + expect(firstResult).to.deep.equal({ + status: 'success', + userInfo: { sub: '1234' }, + }); + expect(firstResult).to.deep.equal(secondResult); expect(store.getState()).to.have.property('state', 'success'); }); @@ -292,8 +390,10 @@ describe('atlasSignInReducer', function () { entrypoint: 'assistant-tool-atlas-connection-error-debugger', }) ); - expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { + expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', + attempt: 1, + previousOutcome: null, }); }); @@ -310,8 +410,10 @@ describe('atlasSignInReducer', function () { track, }); await store.dispatch(performSignInAttempt()); - expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { + expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'unknown', + attempt: 1, + previousOutcome: null, }); }); @@ -332,6 +434,165 @@ describe('atlasSignInReducer', function () { }); }); + describe('sign in attempt tracking (retries)', function () { + let clock: Sinon.SinonFakeTimers; + const ENTRYPOINT = 'assistant-tool-atlas-connection-error-debugger'; + + beforeEach(function () { + clock = sandbox.useFakeTimers({ shouldAdvanceTime: true }); + sandbox.replaceGetter(compassComponents, 'openToast', () => + sandbox.stub() + ); + }); + + afterEach(function () { + clock.restore(); + sandbox.restore(); + }); + + function startedCalls(track: Sinon.SinonStub) { + return track + .getCalls() + .filter((call) => call.args[0] === 'Atlas Sign In Started') + .map((call) => call.args[1]); + } + + async function attemptThenTimeout(store: any) { + const attemptPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await attemptPromise; + } + + it('first attempt, previousOutcome is null', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox.stub().resolves(false), + signIn: sandbox.stub().resolves({ sub: '1234' }), + getUserInfo: sandbox.stub().resolves({ sub: '1234' }), + } as any, + track, + }); + + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + ]); + }); + + it('after a timeout, retrying increases attempt number and fill previousOutcome', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason)); + }); + }), + } as any, + track, + }); + + await attemptThenTimeout(store); + + const secondPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'timed-out' }, + ]); + + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await secondPromise; + }); + + it('after a cancel, reports the previous outcome as canceled when retrying', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason)); + }); + }), + } as any, + track, + }); + + const c = new AbortController(); + const firstPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT, signal: c.signal }) + ); + await clock.tickAsync(0); + c.abort(new Error('user canceled')); + await firstPromise; + + const secondPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'canceled' }, + ]); + + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await secondPromise; + }); + + it('after a failure, reports the previous outcome as error when retrying', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox.stub().resolves(false), + signIn: sandbox.stub().rejects(new Error('Sign in failed')), + getUserInfo: sandbox.stub().resolves({ sub: '1234' }), + } as any, + track, + }); + + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'error' }, + ]); + }); + + it('resets the attempt number after a successful sign in', function () { + const store = configureStore({ atlasAuthService: {} as any }); + + store.dispatch({ + type: 'atlas-service/atlas-signin/AttemptStart', + id: 1, + }); + store.dispatch({ + type: 'atlas-service/atlas-signin/AtlasSignInError', + error: 'some error', + }); + expect(store.getState()).to.have.property('attemptNumber', 2); + + store.dispatch({ + type: 'atlas-service/atlas-signin/AtlasSignInSuccess', + userInfo: { sub: '1234' }, + }); + expect(store.getState()).to.have.property('attemptNumber', 1); + }); + }); + describe('signOut', function () { let openToastStub: Sinon.SinonStub; diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index 3a8f8e2d96f..b3b575beeed 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -20,6 +20,7 @@ export type AtlasSignInState = { error: string | null; // For managing attempt state that doesn't belong in the store currentAttemptId: number | null; + attemptNumber: number; } & ( | { state: @@ -28,12 +29,19 @@ export type AtlasSignInState = { | 'unauthenticated' | 'in-progress' | 'error' - | 'canceled'; + | 'canceled' + | 'timed-out'; userInfo: null; } | { state: 'success'; userInfo: AtlasUserInfo } ); +export type SignInAttemptResult = + | { status: 'success'; userInfo: AtlasUserInfo } + | { status: 'timed-out' } + | { status: 'canceled'; reason?: unknown } + | { status: 'error'; error: Error }; + export type AtlasSignInThunkAction< R, A extends AnyAction = AnyAction @@ -44,6 +52,18 @@ export type AtlasSignInThunkAction< A >; +class TimeoutError extends Error { + constructor() { + super('Sign in timed out'); + } +} + +class CanceledError extends Error { + constructor() { + super('Sign in canceled'); + } +} + // @ts-expect-error TODO(COMPASS-10124): replace enums with const kv objects export const enum AtlasSignInActions { RestoringStart = 'atlas-service/atlas-signin/StartRestoring', @@ -55,6 +75,7 @@ export const enum AtlasSignInActions { Success = 'atlas-service/atlas-signin/AtlasSignInSuccess', Error = 'atlas-service/atlas-signin/AtlasSignInError', Cancel = 'atlas-service/atlas-signin/AtlasSignInCancel', + TimedOut = 'atlas-service/atlas-signin/AtlasSignInTimedOut', TokenRefreshFailed = 'atlas-service/atlas-signin/TokenRefreshFailed', SignedOut = 'atlas-service/atlas-signin/SignedOut', } @@ -106,22 +127,26 @@ export type AtlasSignInSignedOutAction = { export type AtlasSignInCancelAction = { type: AtlasSignInActions.Cancel }; +export type AtlasSignInTimedOutAction = { type: AtlasSignInActions.TimedOut }; + const INITIAL_STATE = { state: 'initial' as const, userInfo: null, error: null, isModalOpen: false, currentAttemptId: null, + attemptNumber: 1, }; type AttemptState = { id: number; controller: AbortController; - promise: Promise; - resolve: (userInfo: AtlasUserInfo) => void; - reject: (reason?: any) => void; + promise: Promise; + resolve: (attemptResult: SignInAttemptResult) => void; }; +export const SIGN_IN_TIMEOUT_MS = 2 * 60 * 1000; // 2 Minutes + // Exported for testing purposes only export const AttemptStateMap = new Map(); @@ -131,21 +156,13 @@ function getAttempt(id?: number | null): AttemptState { if (!id) { id = ++attemptId; const controller = new AbortController(); - let resolve; - let reject; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; + const { promise, resolve } = Promise.withResolvers(); + AttemptStateMap.set(id, { + id, + controller, + promise, + resolve, }); - if (resolve && reject) { - AttemptStateMap.set(id, { - id, - controller, - promise, - resolve: resolve, - reject: reject, - }); - } } const attemptState = AttemptStateMap.get(id); if (!attemptState) { @@ -209,6 +226,7 @@ const reducer: Reducer = ( return { ...state, currentAttemptId: action.id, + attemptNumber: state.attemptNumber + 1, }; } @@ -232,6 +250,7 @@ const reducer: Reducer = ( userInfo: action.userInfo, error: null, isModalOpen: false, + attemptNumber: 1, }; } @@ -246,7 +265,21 @@ const reducer: Reducer = ( } if (isAction(action, AtlasSignInActions.Cancel)) { - return { ...INITIAL_STATE, state: 'canceled' }; + return { + ...INITIAL_STATE, + state: 'canceled', + attemptNumber: state.attemptNumber, + }; + } + + if ( + isAction(action, AtlasSignInActions.TimedOut) + ) { + return { + ...INITIAL_STATE, + state: 'timed-out', + attemptNumber: state.attemptNumber, + }; } if ( @@ -303,8 +336,10 @@ const startAttempt = (fn: () => void): AtlasSignInThunkAction => { "Can't start sign in with prompt while another sign in attempt is in progress" ); } + const attempt = getAttempt(); dispatch({ type: AtlasSignInActions.AttemptStart, id: attempt.id }); + attempt.promise .finally(() => { dispatch({ type: AtlasSignInActions.AttemptEnd, id: attempt.id }); @@ -324,22 +359,29 @@ export const performSignInAttempt = ({ }: { signal?: AbortSignal; entrypoint?: AtlasSignInEntrypoint; -} = {}): AtlasSignInThunkAction> => { +} = {}): AtlasSignInThunkAction> => { return async (dispatch, getState, { track }) => { // Nothing to do if we already signed in - const { state, userInfo, currentAttemptId } = getState(); + const { state, userInfo, currentAttemptId, attemptNumber } = getState(); if (state === 'success') { - return userInfo; + return { status: 'success', userInfo }; } if (currentAttemptId) { return getAttempt(currentAttemptId).promise; } - track('Atlas Sign In Started', { entrypoint }); + track('Atlas Sign In Started', { + entrypoint, + attempt: attemptNumber, + previousOutcome: + state === 'error' || state === 'canceled' || state === 'timed-out' + ? state + : null, + }); const attempt = dispatch( startAttempt(() => { - void dispatch(signIn()); + void dispatch(signIn({ entrypoint })); }) ); signal?.addEventListener('abort', () => { @@ -352,63 +394,98 @@ export const performSignInAttempt = ({ /** * Sign into Atlas. To be called when the user isn't signed in yet. */ -export const signIn = (): AtlasSignInThunkAction> => { - return async (dispatch, getState, { atlasAuthService }) => { +export const signIn = + ({ + entrypoint, + }: { + entrypoint: AtlasSignInEntrypoint; + }): AtlasSignInThunkAction> => + async (dispatch, getState, { atlasAuthService, track }) => { const { - controller: { signal }, + id: currentAttemptId, + controller, resolve, - reject, } = getAttempt(getState().currentAttemptId); dispatch({ type: AtlasSignInActions.Start }); + const signal = controller.signal; + let timeoutId: ReturnType | undefined; try { throwIfAborted(signal); let userInfo; - if (await atlasAuthService.isAuthenticated({ signal })) { - userInfo = await atlasAuthService.getUserInfo({ signal }); - } else { - userInfo = await atlasAuthService.signIn({ - signal, + + const doSignIn = async () => { + if (await atlasAuthService.isAuthenticated({ signal })) { + userInfo = await atlasAuthService.getUserInfo({ signal }); + } else { + userInfo = await atlasAuthService.signIn({ + signal, + }); + } + openToast('atlas-sign-in-success', { + variant: 'success', + title: `Atlas sign in successful`, + timeout: 10_000, }); - } - openToast('atlas-sign-in-success', { - variant: 'success', - title: `Atlas sign in successful`, - timeout: 10_000, + dispatch({ type: AtlasSignInActions.Success, userInfo }); + resolve({ userInfo, status: 'success' }); + }; + const timeoutPromise = new Promise((_resolve, reject) => { + timeoutId = setTimeout(() => { + controller.abort(new TimeoutError()); + reject(new TimeoutError()); + }, SIGN_IN_TIMEOUT_MS); }); - dispatch({ type: AtlasSignInActions.Success, userInfo }); - AttemptStateMap.clear(); - resolve(userInfo); + + await Promise.race([doSignIn(), timeoutPromise]); } catch (err) { - // Only handle error if sign in wasn't aborted by the user, otherwise it - // was already handled in `cancelSignIn` action if (signal.aborted) { - return; + // the canceled flow must be tracked outside of the signIn function + // as it can be triggered by an external caller. + if (signal.reason instanceof CanceledError) { + return; + } else if (signal.reason instanceof TimeoutError) { + openToast('atlas-timed-out', { + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + timeout: 5000, + }); + dispatch({ type: AtlasSignInActions.TimedOut }); + track('Atlas Sign In Timed Out', { entrypoint }); + resolve({ status: 'timed-out' }); + } + } else { + openToast('atlas-sign-in-error', { + variant: 'important', + title: 'Sign in failed', + description: (err as Error).message, + }); + dispatch({ + type: AtlasSignInActions.Error, + error: (err as Error).message, + }); + + resolve({ status: 'error', error: err as Error }); } - openToast('atlas-sign-in-error', { - variant: 'important', - title: 'Sign in failed', - description: (err as Error).message, - }); - dispatch({ - type: AtlasSignInActions.Error, - error: (err as Error).message, - }); - reject(err); + } finally { + AttemptStateMap.delete(currentAttemptId); + // if the timeout is not cleared the promise will be dangling around + clearTimeout(timeoutId); } }; -}; export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { - return (dispatch, getState) => { + return (dispatch, getState, { track }) => { // Can't cancel sign in after the flow was finished indicated by current // attempt id being set to null if (getState().currentAttemptId === null) { return; } const attempt = getAttempt(getState().currentAttemptId); - attempt.controller.abort(); - attempt.reject(reason ?? attempt.controller.signal.reason); + attempt.controller.abort(new CanceledError()); + attempt.resolve({ status: 'canceled', reason }); + AttemptStateMap.delete(attempt.id); dispatch({ type: AtlasSignInActions.Cancel }); + track('Atlas Sign In Canceled', {}); }; }; diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx index 63e3fc5e212..ca7bfcbaaea 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx @@ -5,9 +5,9 @@ import { renderHook } from '@mongodb-js/testing-library-compass'; import type { AnyAction } from 'redux'; import { AtlasSignInStoreContext, - useAtlasSignedInUser, - useIsAtlasSignInStateResolved, + useAtlasSignInStatus, } from './atlas-signin-store-context'; +import type { AtlasSignInStatus } from './atlas-signin-store-context'; import { AtlasSignInActions } from './atlas-signin-reducer'; import { configureStore } from './atlas-signin-store'; @@ -23,65 +23,74 @@ function renderWithState(actions: AnyAction[]) { ); return { store, - isResolved: renderHook(() => useIsAtlasSignInStateResolved(), { wrapper }) - .result, - signedInUser: renderHook(() => useAtlasSignedInUser(), { wrapper }).result, + status: renderHook(() => useAtlasSignInStatus(), { wrapper }).result, }; } +const USER = { sub: '1234' }; const RESTORING = [{ type: AtlasSignInActions.RestoringStart }]; const SUCCESS = [ { type: AtlasSignInActions.RestoringStart }, - { type: AtlasSignInActions.RestoringSuccess, userInfo: { sub: '1234' } }, + { type: AtlasSignInActions.RestoringSuccess, userInfo: USER }, ]; -describe('useIsAtlasSignInStateResolved', function () { - const cases: [string, AnyAction[], boolean][] = [ - ['initial', [], false], - ['restoring', RESTORING, false], - // A manual sign in attempt started while restoring makes the reducer - // discard the restoring result, so until this attempt settles we still - // don't know whether the user is signed in. - ['in-progress', [...RESTORING, { type: AtlasSignInActions.Start }], false], +describe('useAtlasSignInStatus', function () { + const cases: [string, AnyAction[], AtlasSignInStatus][] = [ + ['initial', [], { user: null, state: 'initial' }], + ['restoring', RESTORING, { user: null, state: 'restoring' }], + [ + 'in-progress', + [{ type: AtlasSignInActions.Start }], + { user: null, state: 'in-progress' }, + ], [ 'unauthenticated', [...RESTORING, { type: AtlasSignInActions.RestoringFailed }], - true, + { user: null, state: 'unauthenticated' }, ], - ['success', SUCCESS, true], + ['success', SUCCESS, { user: USER, state: 'success' }], [ 'error', [ { type: AtlasSignInActions.Start }, { type: AtlasSignInActions.Error, error: 'Whoops!' }, ], - true, + { user: null, state: 'error' }, + ], + [ + 'canceled', + [{ type: AtlasSignInActions.Cancel }], + { user: null, state: 'canceled' }, + ], + [ + 'timed-out', + [{ type: AtlasSignInActions.TimedOut }], + { user: null, state: 'timed-out' }, ], - ['canceled', [{ type: AtlasSignInActions.Cancel }], true], ]; for (const [state, actions, expected] of cases) { - it(`should return ${String( - expected - )} for the '${state}' state`, function () { - const { store, isResolved } = renderWithState(actions); + it(`reports the status for the '${state}' state`, function () { + const { store, status } = renderWithState(actions); expect(store.getState()).to.have.property('state', state); - expect(isResolved.current).to.eq(expected); + expect(status.current).to.deep.equal(expected); }); } - it('should not report a resolved state while a sign in started during restore is still in flight', function () { - const { store, isResolved, signedInUser } = renderWithState([ + it('does not report a resolved or signed in state while a sign in started during restore is still in flight', function () { + const { store, status } = renderWithState([ ...RESTORING, { type: AtlasSignInActions.Start }, // The restore finishes after the manual attempt started, the reducer - // ignores it - { type: AtlasSignInActions.RestoringSuccess, userInfo: { sub: '1234' } }, + // ignores it. + { type: AtlasSignInActions.RestoringSuccess, userInfo: USER }, ]); expect(store.getState()).to.have.property('state', 'in-progress'); // The user looks signed out here, so anything acting on that (telemetry, - // for example) has to wait for the state to resolve - expect(signedInUser.current).to.eq(null); - expect(isResolved.current).to.eq(false); + // for example) has to wait for the state to resolve. + expect(status.current).to.deep.equal({ + user: null, + state: 'in-progress', + }); }); }); diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.tsx index ed5d3b9a686..3ec264808b6 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.tsx @@ -2,10 +2,15 @@ import React, { useMemo } from 'react'; import { performSignInAttempt, signOut, + type SignInAttemptResult, type AtlasSignInState, } from './atlas-signin-reducer'; import type { ReactReduxContextValue, TypedUseSelectorHook } from 'react-redux'; -import { createDispatchHook, createSelectorHook } from 'react-redux'; +import { + createDispatchHook, + createSelectorHook, + shallowEqual, +} from 'react-redux'; import type { ThunkDispatch } from 'redux-thunk'; import type { AnyAction } from 'redux'; import type { AtlasAuthService, AtlasUserInfo } from '../provider'; @@ -33,24 +38,26 @@ const useDispatch: () => AtlasSignInDispatch = createDispatchHook( AtlasSignInStoreContext ); -export function useAtlasSignedInUser(): AtlasUserInfo | null { - return useSelector((state) => - state.state === 'success' ? state.userInfo : null - ); -} +export type AtlasSignInStatus = { + user: AtlasUserInfo | null; + state: + | 'initial' + | 'restoring' + | 'unauthenticated' + | 'in-progress' + | 'error' + | 'canceled' + | 'timed-out' + | 'success'; +}; -/** - * Whether we know yet if the user is signed in. The signed in state is restored - * asynchronously on startup, so `useAtlasSignedInUser` returns `null` for an - * already signed in user until that finishes. Anything that shouldn't act on a - * false "signed out" (reporting telemetry, for example) should wait for this. - */ -export function useIsAtlasSignInStateResolved(): boolean { +export function useAtlasSignInStatus() { return useSelector( - (state) => - state.state !== 'initial' && - state.state !== 'restoring' && - state.state !== 'in-progress' + (s): AtlasSignInStatus => ({ + user: s.state === 'success' ? s.userInfo : null, + state: s.state, + }), + shallowEqual ); } @@ -58,7 +65,7 @@ export type AtlasLoginActions = { signOut: () => Promise; signIn: (opts?: { entrypoint?: AtlasSignInEntrypoint; - }) => Promise; + }) => Promise; }; export function useAtlasLoginActions(): AtlasLoginActions { diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx index b8670be6b64..8f73198c8f0 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx @@ -18,18 +18,41 @@ import { AtlasToolCallMessage } from './atlas-tool-call-message'; class FakeAtlasAuthService { private user: AtlasUserInfo | null; public signIn: sinon.SinonStub; + public resolveSignIn: () => void = () => {}; constructor({ signedIn = false, signInSucceeds = true, - }: { signedIn?: boolean; signInSucceeds?: boolean } = {}) { + deferSignIn = false, + }: { + signedIn?: boolean; + signInSucceeds?: boolean; + deferSignIn?: boolean; + } = {}) { this.user = signedIn ? { sub: 'user-1' } : null; this.signIn = sinon.stub().callsFake(() => { - if (!signInSucceeds) { - return Promise.reject(new Error('sign-in failed')); + const complete = () => { + if (!signInSucceeds) { + throw new Error('sign-in failed'); + } + this.user = { sub: 'user-1' }; + return this.user; + }; + if (!deferSignIn) { + return signInSucceeds + ? Promise.resolve(complete()) + : Promise.reject(new Error('sign-in failed')); } - this.user = { sub: 'user-1' }; - return Promise.resolve(this.user); + // Do not resolve immediately so we can test the in-progress flows + return new Promise((resolve, reject) => { + this.resolveSignIn = () => { + try { + resolve(complete()); + } catch (err) { + reject(err as Error); + } + }; + }); }); } @@ -73,13 +96,19 @@ describe('AtlasToolCallMessage', function () { { signedIn = false, signInSucceeds = true, - }: { signedIn?: boolean; signInSucceeds?: boolean } = {} + deferSignIn = false, + }: { + signedIn?: boolean; + signInSucceeds?: boolean; + deferSignIn?: boolean; + } = {} ) { const onApprove = sinon.stub(); const onDeny = sinon.stub(); const atlasAuthService = new FakeAtlasAuthService({ signedIn, signInSucceeds, + deferSignIn, }); const { renderWithConnections } = createPluginTestHelpers( // eslint-disable-next-line @typescript-eslint/no-unsafe-argument @@ -155,6 +184,8 @@ describe('AtlasToolCallMessage', function () { await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', + attempt: 1, + previousOutcome: null, }); }); }); @@ -221,6 +252,60 @@ describe('AtlasToolCallMessage', function () { }); }); + describe('while sign in is in progress', function () { + async function startSignIn(opts: { signedIn?: boolean } = {}) { + const rendered = renderMessage({}, { ...opts, deferSignIn: true }); + const buttonLabel = opts.signedIn ? 'Run' : 'Connect to Atlas'; + await waitFor(() => { + expect(screen.getByText(buttonLabel)).to.exist; + }); + userEvent.click(screen.getByText(buttonLabel)); + // Wait for the store to move into the 'in-progress' state, surfaced as the + // connecting message. + await waitFor(() => { + expect( + screen.getByText('Connecting with Atlas to debug this connection') + ).to.exist; + }); + return rendered; + } + + it('shows the connecting message', async function () { + const { atlasAuthService, onApprove } = await startSignIn(); + + expect(screen.getByText('Connecting with Atlas to debug this connection')) + .to.exist; + + // Let the pending sign in settle so we don't leave a dangling attempt + atlasAuthService.resolveSignIn(); + await waitFor(() => { + expect(onApprove).to.have.been.called; + }); + }); + + it('hides the approval action buttons', async function () { + const { atlasAuthService, onApprove } = await startSignIn(); + + expect(screen.queryByText('Connect to Atlas')).to.not.exist; + expect(screen.queryByText('Skip')).to.not.exist; + + atlasAuthService.resolveSignIn(); + await waitFor(() => { + expect(onApprove).to.have.been.called; + }); + }); + + it('resolves the approval once sign in completes', async function () { + const { onApprove, atlasAuthService } = await startSignIn(); + + atlasAuthService.resolveSignIn(); + + await waitFor(() => { + expect(onApprove).to.have.been.calledOnceWith('approval-1', true); + }); + }); + }); + describe('resolved states', function () { it('shows "Ran" title and hides the action buttons when run', function () { const { container } = renderMessage( diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index 65693c2037c..d3f659640dc 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -22,8 +22,7 @@ import { import type { AtlasSignInEntrypoint } from '@mongodb-js/compass-telemetry'; import { useAtlasLoginActions, - useAtlasSignedInUser, - useIsAtlasSignInStateResolved, + useAtlasSignInStatus, } from '@mongodb-js/atlas-service/provider'; import { CustomToolResult } from './custom-tool-result'; import { getToolCallTitle } from './tool-call-title'; @@ -69,6 +68,21 @@ This is read-only and won't change your cluster.`; ); } +// TODO(COMPASS-11044): update texts to be generic +function getApprovalMessage( + isSignInInProgress: boolean, + isUserSignedIn: boolean +) { + if (isUserSignedIn) { + return 'Run Atlas to debug this connection?'; + } + if (isSignInInProgress) { + return 'Connecting with Atlas to debug this connection'; + } + + return 'Connect with Atlas to debug this connection?'; +} + export const AtlasToolCallMessage: React.FunctionComponent< AtlasToolCallMessageProps > = ({ toolCall, connectionInfo, onApprove, onDeny }) => { @@ -76,10 +90,16 @@ export const AtlasToolCallMessage: React.FunctionComponent< const toolDisplayName = getToolDisplayName(toolCall.type); const isAwaitingApproval = toolCallState === 'idle' && !!toolCall.approval; const approvalId = toolCall.approval?.id; - const isUserSignedIn = !!useAtlasSignedInUser(); + const atlasSignInStatus = useAtlasSignInStatus(); + const isUserSignedIn = !!atlasSignInStatus.user; + const isSignInInProgress = atlasSignInStatus.state === 'in-progress'; const { signIn } = useAtlasLoginActions(); const track = useTelemetry(); - const isSignInStateResolved = useIsAtlasSignInStateResolved(); + + const isSignInStateResolved = + atlasSignInStatus.state !== 'initial' && + atlasSignInStatus.state !== 'restoring' && + atlasSignInStatus.state !== 'in-progress'; // The card re-renders on every state change, so we only report the prompt the // first time it's actually offered to a signed out user. We also wait for the @@ -119,7 +139,20 @@ export const AtlasToolCallMessage: React.FunctionComponent< const handleAtlasToolApproval = useCallback( (approvalId: string) => { signIn({ entrypoint: getSignInEntrypoint(toolCall.type) }) - .then((userInfo) => onApprove(approvalId, !!userInfo)) + .then((result) => { + switch (result.status) { + case 'success': + onApprove(approvalId, true); + break; + // If sign in timed out, give the user a new chance instead of + // rejecting the tool + case 'timed-out': + break; + default: + onApprove(approvalId, false); + break; + } + }) .catch(() => onApprove(approvalId, false)); }, [signIn, onApprove, toolCall.type] @@ -148,19 +181,18 @@ export const AtlasToolCallMessage: React.FunctionComponent< toolDisplayName ); - // TODO(COMPASS-11044): update texts to be generic - const approvalMessage = isUserSignedIn - ? 'Run Atlas to debug this connection?' - : 'Connect with Atlas to debug this connection?'; - + const approvalMessage = getApprovalMessage( + isSignInInProgress, + isUserSignedIn + ); // TODO COMPASS-10973: don't render actions if there's no approvalId. return ( <> ; +/** + * This event is fired when the user aborts the current sign in attempt. + * + * @category Atlas + */ +type AtlasSignInCanceledEvent = CommonEvent<{ + name: 'Atlas Sign In Canceled'; + payload: Record; +}>; + /** * This event is fired when user successfully signed in to their Atlas account * @@ -204,6 +225,22 @@ type AtlasSignInErrorEvent = CommonEvent<{ }; }>; +/** + * This event is fired when the user does not complete the sign in to their Atlas + * account on time. + * + * @category Atlas + */ +type AtlasSignInTimedOutEvent = CommonEvent<{ + name: 'Atlas Sign In Timed Out'; + payload: { + /** + * The surface of the application the sign in was triggered from. + */ + entrypoint: AtlasSignInEntrypoint; + }; +}>; + /** * This event is fired when user signed out from their Atlas account. * @@ -4192,7 +4229,9 @@ export type TelemetryEvent = | ApplicationLaunchedEvent | AtlasLinkClickedEvent | AtlasSearchIndexesForViewLinkClickedEvent + | AtlasSignInCanceledEvent | AtlasSignInErrorEvent + | AtlasSignInTimedOutEvent | AtlasSignInPromptShownEvent | AtlasSignInStartedEvent | AtlasSignInSuccessEvent