Skip to content

fix: Only warn if multiple clients share the same storage-key #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ async function lockNoOp<R>(name: string, acquireTimeout: number, fn: () => Promi
const GLOBAL_JWKS: { [storageKey: string]: { cachedAt: number; jwks: { keys: JWK[] } } } = {}

export default class GoTrueClient {
private static nextInstanceID = 0
private static nextInstanceID: Record<string, number> = {}

private instanceID: number

Expand Down Expand Up @@ -238,24 +238,26 @@ export default class GoTrueClient {
* Create a new client for use in the browser.
*/
constructor(options: GoTrueClientOptions) {
this.instanceID = GoTrueClient.nextInstanceID
GoTrueClient.nextInstanceID += 1

if (this.instanceID > 0 && isBrowser()) {
console.warn(
'Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key.'
)
}

const settings = { ...DEFAULT_OPTIONS, ...options }
this.storageKey = settings.storageKey

this.instanceID = GoTrueClient.nextInstanceID[this.storageKey] ?? 0
GoTrueClient.nextInstanceID[this.storageKey] = this.instanceID + 1

this.logDebugMessages = !!settings.debug
if (typeof settings.debug === 'function') {
this.logger = settings.debug
}

if (this.instanceID > 0 && isBrowser()) {
const message = `${this._logPrefix()} Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key.`
console.warn(message)
if (this.logDebugMessages) {
console.trace(message)
}
}

this.persistSession = settings.persistSession
this.storageKey = settings.storageKey
this.autoRefreshToken = settings.autoRefreshToken
this.admin = new GoTrueAdminApi({
url: settings.url,
Expand Down Expand Up @@ -334,12 +336,16 @@ export default class GoTrueClient {
this.initialize()
}

private _logPrefix(): string {
return (
'GoTrueClient@' +
`${this.storageKey}:${this.instanceID} (${version}) ${new Date().toISOString()}`
)
}

private _debug(...args: any[]): GoTrueClient {
if (this.logDebugMessages) {
this.logger(
`GoTrueClient@${this.instanceID} (${version}) ${new Date().toISOString()}`,
...args
)
this.logger(this._logPrefix(), ...args)
}

return this
Expand Down
95 changes: 94 additions & 1 deletion test/GoTrueClient.browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
* @jest-environment jsdom
*/

import { autoRefreshClient, getClientWithSpecificStorage, pkceClient } from './lib/clients'
import {
autoRefreshClient,
getClientWithSpecificStorage,
getClientWithSpecificStorageKey,
pkceClient,
} from './lib/clients'
import { mockUserCredentials } from './lib/utils'

// Add structuredClone polyfill for jsdom
Expand Down Expand Up @@ -98,6 +103,94 @@ describe('GoTrueClient in browser environment', () => {
expect(signinError).toBeNull()
expect(signinData?.session).toBeDefined()
})

it('should warn when two clients are created with the same storage key', () => {
let consoleWarnSpy
let consoleTraceSpy
try {
consoleWarnSpy = jest.spyOn(console, 'warn')
consoleTraceSpy = jest.spyOn(console, 'trace')
getClientWithSpecificStorageKey('same-storage-key')
getClientWithSpecificStorageKey('same-storage-key')
expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringMatching(
/GoTrueClient@same-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
)
)
expect(consoleTraceSpy).not.toHaveBeenCalled()
} finally {
consoleWarnSpy?.mockRestore()
consoleTraceSpy?.mockRestore()
}
})

it('should warn & trace when two clients are created with the same storage key and debug is enabled', () => {
let consoleWarnSpy
let consoleTraceSpy
try {
consoleWarnSpy = jest.spyOn(console, 'warn')
consoleTraceSpy = jest.spyOn(console, 'trace')
getClientWithSpecificStorageKey('identical-storage-key')
getClientWithSpecificStorageKey('identical-storage-key', { debug: true })
expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringMatching(
/GoTrueClient@identical-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
)
)
expect(consoleTraceSpy).toHaveBeenCalledWith(
expect.stringMatching(
/GoTrueClient@identical-storage-key:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
)
)
} finally {
consoleWarnSpy?.mockRestore()
consoleTraceSpy?.mockRestore()
}
})

it('should not warn when two clients are created with differing storage keys', () => {
let consoleWarnSpy
let consoleTraceSpy
try {
consoleWarnSpy = jest.spyOn(console, 'warn')
consoleTraceSpy = jest.spyOn(console, 'trace')
getClientWithSpecificStorageKey('first-storage-key')
getClientWithSpecificStorageKey('second-storage-key')
expect(consoleWarnSpy).not.toHaveBeenCalled()
expect(consoleTraceSpy).not.toHaveBeenCalled()
} finally {
consoleWarnSpy?.mockRestore()
consoleTraceSpy?.mockRestore()
}
})

it('should warn only when a second client with a duplicate key is created', () => {
let consoleWarnSpy
let consoleTraceSpy
try {
consoleWarnSpy = jest.spyOn(console, 'warn')
consoleTraceSpy = jest.spyOn(console, 'trace')
getClientWithSpecificStorageKey('test-storage-key1')
expect(consoleWarnSpy).not.toHaveBeenCalled()
getClientWithSpecificStorageKey('test-storage-key2')
expect(consoleWarnSpy).not.toHaveBeenCalled()
getClientWithSpecificStorageKey('test-storage-key3')
expect(consoleWarnSpy).not.toHaveBeenCalled()
getClientWithSpecificStorageKey('test-storage-key2')
expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringMatching(
/GoTrueClient@test-storage-key2:1 .* Multiple GoTrueClient instances detected in the same browser context. It is not an error, but this should be avoided as it may produce undefined behavior when used concurrently under the same storage key./
)
)
expect(consoleTraceSpy).not.toHaveBeenCalled()
} finally {
consoleWarnSpy?.mockRestore()
consoleTraceSpy?.mockRestore()
}
})
})

describe('Callback URL handling', () => {
Expand Down
15 changes: 14 additions & 1 deletion test/lib/clients.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import jwt from 'jsonwebtoken'
import { GoTrueAdminApi, GoTrueClient } from '../../src/index'
import { GoTrueAdminApi, GoTrueClient, type GoTrueClientOptions } from '../../src/index'
import { SupportedStorage } from '../../src/lib/types'

export const SIGNUP_ENABLED_AUTO_CONFIRM_OFF_PORT = 9999
Expand Down Expand Up @@ -156,3 +156,16 @@ export function getClientWithSpecificStorage(storage: SupportedStorage) {
storage,
})
}

export function getClientWithSpecificStorageKey(
storageKey: string,
opts: GoTrueClientOptions = {}
) {
return new GoTrueClient({
url: GOTRUE_URL_SIGNUP_ENABLED_AUTO_CONFIRM_ON,
autoRefreshToken: false,
persistSession: true,
storageKey,
...opts,
})
}