Skip to content
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

feat: add experimental lock option with no-op default #731

Closed
wants to merge 1 commit into from
Closed
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
61 changes: 55 additions & 6 deletions src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ import type {
MFAChallengeAndVerifyParams,
ResendParams,
AuthFlowType,
LockFunc,
} from './lib/types'

polyfillGlobalThis() // Make "globalThis" available

const DEFAULT_OPTIONS: Omit<Required<GoTrueClientOptions>, 'fetch' | 'storage'> = {
const DEFAULT_OPTIONS: Omit<Required<GoTrueClientOptions>, 'fetch' | 'storage' | 'lock'> = {
url: GOTRUE_URL,
storageKey: STORAGE_KEY,
autoRefreshToken: true,
Expand All @@ -99,6 +100,10 @@ const AUTO_REFRESH_TICK_DURATION = 30 * 1000
* A token refresh will be attempted this many ticks before the current session expires. */
const AUTO_REFRESH_TICK_THRESHOLD = 3

async function lockNoOp<R>(name: string, acquireTimeout: number, fn: () => Promise<R>): Promise<R> {
return await fn()
}

export default class GoTrueClient {
private static nextInstanceID = 0

Expand Down Expand Up @@ -146,6 +151,7 @@ export default class GoTrueClient {
[key: string]: string
}
protected fetch: Fetch
protected lock: LockFunc

/**
* Used to broadcast state change events to other tabs listening.
Expand Down Expand Up @@ -174,6 +180,7 @@ export default class GoTrueClient {
this.autoRefreshToken = settings.autoRefreshToken
this.persistSession = settings.persistSession
this.storage = settings.storage || localStorageAdapter
this.lock = settings.lock || lockNoOp
this.admin = new GoTrueAdminApi({
url: settings.url,
headers: settings.headers,
Expand Down Expand Up @@ -775,6 +782,28 @@ export default class GoTrueClient {
})
}

private async _acquireLock<R>(
name: string,
acquireTimeout: number,
fn: () => Promise<R>
): Promise<R> {
try {
this._debug('#_acquireLock', name, 'start')

return this.lock(`lock:${this.storageKey}:${name}`, acquireTimeout, async () => {
try {
this._debug('#_acquireLock', name, 'acquired')

return await fn()
} finally {
this._debug('#_acquireLock', name, 'released')
}
})
} finally {
this._debug('#_acquireLock', name, 'end')
}
}

/**
* Use instead of {@link #getSession} inside the library. It is
* semantically usually what you want, as getting a session involves some
Expand Down Expand Up @@ -802,14 +831,27 @@ export default class GoTrueClient {
}
error: null
}
) => Promise<R>
) => Promise<R>,
acquireTimeout: number = -1
): Promise<R> {
return await stackGuard('_useSession', async () => {
if (isInStackGuard('_useSession')) {
this._debug('#_useSession', 'recursive use detected')

// the lock should not be recursively held to avoid dead-locks
// the use of __loadSession here is the only correct use of the function!
const result = await this.__loadSession()

return await fn(result)
})
} else {
return await this._acquireLock('_useSession', acquireTimeout, async () => {
return await stackGuard('_useSession', async () => {
// the use of __loadSession here is the only correct use of the function!
const result = await this.__loadSession()

return await fn(result)
})
})
}
}

/**
Expand Down Expand Up @@ -1720,9 +1762,16 @@ export default class GoTrueClient {
if (expiresInTicks <= AUTO_REFRESH_TICK_THRESHOLD) {
await this._callRefreshToken(session.refresh_token)
}
})
}, 0 /* try-lock */)
} catch (e: any) {
console.error('Auto refresh tick failed with error. This is likely a transient error.', e)
if (e.isAcquireTimeout) {
this._debug(
'#_autoRefreshTokenTick()',
'lock is already acquired, skipping for next tick'
)
} else {
console.error('Auto refresh tick failed with error. This is likely a transient error.', e)
Copy link
Contributor

@J0 J0 Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the best way to advise a customer to debug this if they come back to us with an error? Wondering how we'd advise to support how to classify whether whether no action is needed vs it's something we should investigate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's transient, but logging so it's not silent.

}
}
} finally {
this._debug('#_autoRefreshTokenTick()', 'end')
Expand Down
75 changes: 74 additions & 1 deletion src/lib/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ const STACK_GUARD_SUFFIX = `__`
// they all include the function name. So instead of trying to parse the entry,
// we're only looking for the special string `__stack_guard__${guardName}__`.
// Guard names can only be letters with dashes or underscores.
//
//
// Example Firefox stack trace:
// ```
// __stack_guard__EXAMPLE__@debugger eval code:1:55
Expand Down Expand Up @@ -393,3 +393,76 @@ STACK_GUARD_CHECK_FN = async () => {
})
}
}

const LOCAL_CHANNELS: { [name: string]: Set<WrappedBroadcastChannel<any>> } = {}

/**
* Wraps a `BroadcastChannel` for use in environments where it is not
* available, like Node.js.
*/
export class WrappedBroadcastChannel<D> {
private _bc: BroadcastChannel | null = null

private _onmessage: null | ((event: { data: D }) => any) = null

set onmessage(cb: null | ((event: { data: D }) => any)) {
this._onmessage = cb

if (this._bc) {
if (cb) {
this._bc.onmessage = (event) => {
cb(event)
}
} else {
this._bc.onmessage = null
}
}
}

get onmessage() {
return this._onmessage
}

constructor(readonly name: string) {
if (globalThis.BroadcastChannel) {
this._bc = new globalThis.BroadcastChannel(name)
} else {
if (!LOCAL_CHANNELS[name]) {
LOCAL_CHANNELS[name] = new Set()
}

LOCAL_CHANNELS[name].add(this)
}
}

postMessage(data: D) {
if (this._bc) {
this._bc.postMessage(data)
return
}

setTimeout(() => {
LOCAL_CHANNELS[this.name].forEach((ch) => {
if (ch === this) {
return
}

if (ch._onmessage) {
ch._onmessage({ data })
}
})
}, 0)
}

close() {
this.onmessage = null

if (this._bc) {
this._bc.close()
} else {
LOCAL_CHANNELS[this.name].delete(this)
}

this._bc = null
}
}
Loading
Loading