From 90ed4953e9c77877e11da2a2030de13e16ffecf4 Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Mon, 1 Aug 2022 09:41:01 +0200 Subject: [PATCH 1/6] fix: resolve & reset deferred upon refresh error --- src/GoTrueClient.ts | 41 ++++++++++++++++++++++++--------------- test/GoTrueClient.test.ts | 33 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index dbbede882..a14bd753f 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -37,6 +37,14 @@ import type { polyfillGlobalThis() // Make "globalThis" available +type CallRefreshTokenResult = { + session: Session + error: null +} | { + session: null + error: AuthError +} + const DEFAULT_OPTIONS = { url: GOTRUE_URL, storageKey: STORAGE_KEY, @@ -83,10 +91,7 @@ export default class GoTrueClient { protected refreshTokenTimer?: ReturnType // eslint-disable-next-line @typescript-eslint/no-inferrable-types protected networkRetries: number = 0 - protected refreshingDeferred: Deferred<{ - session: Session - error: null - }> | null = null + protected refreshingDeferred: Deferred | null = null /** * Create a new client for use in the browser. @@ -884,17 +889,16 @@ export default class GoTrueClient { } } - private async _callRefreshToken(refresh_token = this.currentSession?.refresh_token) { - try { - // refreshing is already in progress - if (this.refreshingDeferred) { - return await this.refreshingDeferred.promise - } + private async _callRefreshToken( + refresh_token = this.currentSession?.refresh_token + ): Promise { + // refreshing is already in progress + if (this.refreshingDeferred) { + return this.refreshingDeferred.promise + } - this.refreshingDeferred = new Deferred<{ - session: Session - error: null - }>() + try { + this.refreshingDeferred = new Deferred() if (!refresh_token) { throw new Error('No current session.') @@ -910,15 +914,20 @@ export default class GoTrueClient { const result = { session, error: null } this.refreshingDeferred.resolve(result) - this.refreshingDeferred = null return result } catch (error) { if (isAuthError(error)) { - return { session: null, error } + const result = { session: null, error }; + + this.refreshingDeferred?.resolve(result); + + return result; } throw error + } finally { + this.refreshingDeferred = null } } diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index 815acb5ce..923e02a86 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -1,4 +1,5 @@ import { OpenIDConnectCredentials } from '../src' +import { AuthError } from '../src/lib/errors'; import { authClient as auth, authClientWithSession as authWithSession, @@ -133,6 +134,38 @@ describe('GoTrueClient', () => { expect(refreshAccessTokenSpy).toBeCalledTimes(1) }) + test('refreshSession() should reject all pending refresh requests and reset deferred', async () => { + const { email, password } = mockUserCredentials() + refreshAccessTokenSpy.mockImplementationOnce(() => Promise.resolve({ + session: null, + error: new AuthError('Something did not work as expected') + })) + + const { error, session } = await authWithSession.signUp({ + email, + password, + }) + + expect(error).toBeNull() + expect(session).not.toBeNull() + + const [{ session: session1, error: error1 }, { session: session2, error: error2 }] = + await Promise.all([authWithSession.refreshSession(), authWithSession.refreshSession()]) + + expect(error1).toHaveProperty('message') + expect(error2).toHaveProperty('message') + expect(session1).toBeNull() + expect(session2).toBeNull() + + expect(refreshAccessTokenSpy).toBeCalledTimes(1) + + // vreify the deferred has been reset and successive calls can be made + const { session: session3, error: error3 } = await authWithSession.refreshSession() + + expect(error3).toBeNull() + expect(session3).toHaveProperty('access_token') + }) + test('getSessionFromUrl() can only be called from a browser', async () => { const { error, session } = await authWithSession.getSessionFromUrl() From ff228a755cd81d52ced1865274d29bd01b7ee506 Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Mon, 1 Aug 2022 10:12:11 +0200 Subject: [PATCH 2/6] fix: properly handle non AuthError's If error is not an AuthError _callRefrehToken throws the error This commit ads a test and fix for this scenario. --- src/GoTrueClient.ts | 1 + test/GoTrueClient.test.ts | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index a14bd753f..b6ca2660c 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -925,6 +925,7 @@ export default class GoTrueClient { return result; } + this.refreshingDeferred?.reject(error); throw error } finally { this.refreshingDeferred = null diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index 923e02a86..cab5c3262 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -134,7 +134,7 @@ describe('GoTrueClient', () => { expect(refreshAccessTokenSpy).toBeCalledTimes(1) }) - test('refreshSession() should reject all pending refresh requests and reset deferred', async () => { + test('refreshSession() should resolve all pending refresh requests and reset deferred upon AuthError', async () => { const { email, password } = mockUserCredentials() refreshAccessTokenSpy.mockImplementationOnce(() => Promise.resolve({ session: null, @@ -166,6 +166,39 @@ describe('GoTrueClient', () => { expect(session3).toHaveProperty('access_token') }) + test('refreshSession() should reject all pending refresh requests and reset deferred upon any non AuthError', async () => { + const mockError = new Error('Something did not work as expected'); + + const { email, password } = mockUserCredentials() + refreshAccessTokenSpy.mockImplementationOnce(() => Promise.reject(mockError)) + + const { error, session } = await authWithSession.signUp({ + email, + password, + }) + + expect(error).toBeNull() + expect(session).not.toBeNull() + + const [error1, error2] = + await Promise.allSettled([authWithSession.refreshSession(), authWithSession.refreshSession()]) + + expect(error1.status).toEqual('rejected') + expect(error2.status).toEqual('rejected') + + // status === 'rejected' above makes sure it is a PromiseRejectedResult + expect((error1 as PromiseRejectedResult).reason).toEqual(mockError) + expect((error1 as PromiseRejectedResult).reason).toEqual(mockError) + + expect(refreshAccessTokenSpy).toBeCalledTimes(1) + + // vreify the deferred has been reset and successive calls can be made + const { session: session3, error: error3 } = await authWithSession.refreshSession() + + expect(error3).toBeNull() + expect(session3).toHaveProperty('access_token') + }) + test('getSessionFromUrl() can only be called from a browser', async () => { const { error, session } = await authWithSession.getSessionFromUrl() From 176d15c0d6b7819bbe0f9296350d46dffd7d8cbb Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Tue, 9 Aug 2022 11:44:11 +0200 Subject: [PATCH 3/6] chore: move CallRefreshTokenResult to lib/types.ts --- src/GoTrueClient.ts | 9 +-------- src/lib/types.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index 80e4a35ec..d9124a703 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -44,18 +44,11 @@ import type { SignInWithPasswordlessCredentials, AuthResponse, OAuthResponse, + CallRefreshTokenResult, } from './lib/types' polyfillGlobalThis() // Make "globalThis" available -type CallRefreshTokenResult = { - session: Session - error: null -} | { - session: null - error: AuthError -} - const DEFAULT_OPTIONS = { url: GOTRUE_URL, storageKey: STORAGE_KEY, diff --git a/src/lib/types.ts b/src/lib/types.ts index ff8f252d9..05a12d2fc 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -272,3 +272,11 @@ type PromisifyMethods = { } export type SupportedStorage = PromisifyMethods> + +export type CallRefreshTokenResult = { + session: Session + error: null +} | { + session: null + error: AuthError +} From 7e20b31d269aea0221999946890cda488606928f Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Tue, 9 Aug 2022 11:54:34 +0200 Subject: [PATCH 4/6] chore: prettier changes --- src/GoTrueClient.ts | 8 ++++---- src/lib/types.ts | 16 +++++++++------- test/GoTrueClient.test.ts | 38 ++++++++++++++++++++++++++------------ 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/GoTrueClient.ts b/src/GoTrueClient.ts index d9124a703..20d78d320 100644 --- a/src/GoTrueClient.ts +++ b/src/GoTrueClient.ts @@ -803,14 +803,14 @@ export default class GoTrueClient { return result } catch (error) { if (isAuthError(error)) { - const result = { session: null, error }; + const result = { session: null, error } - this.refreshingDeferred?.resolve(result); + this.refreshingDeferred?.resolve(result) - return result; + return result } - this.refreshingDeferred?.reject(error); + this.refreshingDeferred?.reject(error) throw error } finally { this.refreshingDeferred = null diff --git a/src/lib/types.ts b/src/lib/types.ts index 05a12d2fc..0ae2eb77f 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -273,10 +273,12 @@ type PromisifyMethods = { export type SupportedStorage = PromisifyMethods> -export type CallRefreshTokenResult = { - session: Session - error: null -} | { - session: null - error: AuthError -} +export type CallRefreshTokenResult = + | { + session: Session + error: null + } + | { + session: null + error: AuthError + } diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index c6c8901bf..e3bcd34d4 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -1,4 +1,4 @@ -import { AuthError } from '../src/lib/errors'; +import { AuthError } from '../src/lib/errors' import { authClient as auth, authClientWithSession as authWithSession, @@ -117,10 +117,12 @@ describe('GoTrueClient', () => { test('_callRefreshToken() should resolve all pending refresh requests and reset deferred upon AuthError', async () => { const { email, password } = mockUserCredentials() - refreshAccessTokenSpy.mockImplementationOnce(() => Promise.resolve({ - session: null, - error: new AuthError('Something did not work as expected') - })) + refreshAccessTokenSpy.mockImplementationOnce(() => + Promise.resolve({ + session: null, + error: new AuthError('Something did not work as expected'), + }) + ) const { error, session } = await authWithSession.signUp({ email, @@ -131,8 +133,12 @@ describe('GoTrueClient', () => { expect(session).not.toBeNull() const [{ session: session1, error: error1 }, { session: session2, error: error2 }] = - // @ts-expect-error 'Allow access to private _callRefreshToken()' - await Promise.all([authWithSession._callRefreshToken(session?.refresh_token), authWithSession._callRefreshToken(session?.refresh_token)]) + await Promise.all([ + // @ts-expect-error 'Allow access to private _callRefreshToken()' + authWithSession._callRefreshToken(session?.refresh_token), + // @ts-expect-error 'Allow access to private _callRefreshToken()' + authWithSession._callRefreshToken(session?.refresh_token), + ]) expect(error1).toHaveProperty('message') expect(error2).toHaveProperty('message') @@ -143,14 +149,16 @@ describe('GoTrueClient', () => { // vreify the deferred has been reset and successive calls can be made // @ts-expect-error 'Allow access to private _callRefreshToken()' - const { session: session3, error: error3 } = await authWithSession._callRefreshToken(session?.refresh_token) + const { session: session3, error: error3 } = await authWithSession._callRefreshToken( + session?.refresh_token + ) expect(error3).toBeNull() expect(session3).toHaveProperty('access_token') }) test('_callRefreshToken() should reject all pending refresh requests and reset deferred upon any non AuthError', async () => { - const mockError = new Error('Something did not work as expected'); + const mockError = new Error('Something did not work as expected') const { email, password } = mockUserCredentials() refreshAccessTokenSpy.mockImplementationOnce(() => Promise.reject(mockError)) @@ -164,8 +172,12 @@ describe('GoTrueClient', () => { expect(session).not.toBeNull() const [error1, error2] = - // @ts-expect-error 'Allow access to private _callRefreshToken()' - await Promise.allSettled([authWithSession._callRefreshToken(session?.refresh_token), authWithSession._callRefreshToken(session?.refresh_token)]) + await Promise.allSettled([ + // @ts-expect-error 'Allow access to private _callRefreshToken()' + authWithSession._callRefreshToken(session?.refresh_token), + // @ts-expect-error 'Allow access to private _callRefreshToken()' + authWithSession._callRefreshToken(session?.refresh_token), + ]) expect(error1.status).toEqual('rejected') expect(error2.status).toEqual('rejected') @@ -178,7 +190,9 @@ describe('GoTrueClient', () => { // vreify the deferred has been reset and successive calls can be made // @ts-expect-error 'Allow access to private _callRefreshToken()' - const { session: session3, error: error3 } = await authWithSession._callRefreshToken(session?.refresh_token) + const { session: session3, error: error3 } = await authWithSession._callRefreshToken( + session?.refresh_token + ) expect(error3).toBeNull() expect(session3).toHaveProperty('access_token') From 709e3f4691ac368dd421de6df2498fd8eaeea498 Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Tue, 9 Aug 2022 12:08:13 +0200 Subject: [PATCH 5/6] fix: fix ts error introduced with prettier changes --- test/GoTrueClient.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index e3bcd34d4..d02b2184f 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -150,7 +150,7 @@ describe('GoTrueClient', () => { // vreify the deferred has been reset and successive calls can be made // @ts-expect-error 'Allow access to private _callRefreshToken()' const { session: session3, error: error3 } = await authWithSession._callRefreshToken( - session?.refresh_token + session!.refresh_token ) expect(error3).toBeNull() @@ -191,7 +191,7 @@ describe('GoTrueClient', () => { // vreify the deferred has been reset and successive calls can be made // @ts-expect-error 'Allow access to private _callRefreshToken()' const { session: session3, error: error3 } = await authWithSession._callRefreshToken( - session?.refresh_token + session!.refresh_token ) expect(error3).toBeNull() From 828b0b355ef67ac031e02d63476ba70e50450764 Mon Sep 17 00:00:00 2001 From: Stefan Aebischer Date: Tue, 9 Aug 2022 12:39:55 +0200 Subject: [PATCH 6/6] fix: correct typo --- test/GoTrueClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/GoTrueClient.test.ts b/test/GoTrueClient.test.ts index d02b2184f..d47619704 100644 --- a/test/GoTrueClient.test.ts +++ b/test/GoTrueClient.test.ts @@ -147,7 +147,7 @@ describe('GoTrueClient', () => { expect(refreshAccessTokenSpy).toBeCalledTimes(1) - // vreify the deferred has been reset and successive calls can be made + // verify the deferred has been reset and successive calls can be made // @ts-expect-error 'Allow access to private _callRefreshToken()' const { session: session3, error: error3 } = await authWithSession._callRefreshToken( session!.refresh_token