Skip to content

fix: improve auth session sync reliability#22

Open
PJ-Snap wants to merge 1 commit intoTimChild:mainfrom
snap-analytics:pr/auth-session-sync-reliability
Open

fix: improve auth session sync reliability#22
PJ-Snap wants to merge 1 commit intoTimChild:mainfrom
snap-analytics:pr/auth-session-sync-reliability

Conversation

@PJ-Snap
Copy link
Copy Markdown
Contributor

@PJ-Snap PJ-Snap commented Apr 7, 2026

  • Add configurable JWT validation leeway (default 60s) to handle clock drift between Clerk servers and the backend
  • Catch ExpiredTokenError alongside InvalidClaimError/MissingClaimError to prevent crashes on expired tokens
  • Rewrite ClerkSessionSynchronizer JS for reliability:
    • Use useRef to deduplicate rapid calls while remaining reconnect-safe
    • Request fresh tokens with skipCache to avoid near-expiry cached tokens
    • Handle token retrieval failures gracefully (clear session instead of hang)
    • Include all dependencies in useEffect array ([isLoaded, isSignedIn, addEvents, getToken])
  • Add unit tests for expired token handling and JS code correctness
  • Add pythonpath config for pytest to find custom_components

- Add configurable JWT validation leeway (default 60s) to handle clock
  drift between Clerk servers and the backend
- Catch ExpiredTokenError alongside InvalidClaimError/MissingClaimError
  to prevent crashes on expired tokens
- Rewrite ClerkSessionSynchronizer JS for reliability:
  - Use useRef to deduplicate rapid calls while remaining reconnect-safe
  - Request fresh tokens with skipCache to avoid near-expiry cached tokens
  - Handle token retrieval failures gracefully (clear session instead of hang)
  - Include all dependencies in useEffect array ([isLoaded, isSignedIn, addEvents, getToken])
- Add unit tests for expired token handling and JS code correctness
- Add pythonpath config for pytest to find custom_components
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves Clerk auth session synchronization between the frontend and backend by making JWT validation more tolerant of clock drift, handling expired tokens safely, and hardening the frontend session synchronizer logic.

Changes:

  • Add configurable JWT validation leeway (default 60s) and treat expired tokens as a non-crashing validation failure.
  • Rewrite the generated ClerkSessionSynchronizer JS to be more reconnect-safe and to prefer fresh tokens (skipCache).
  • Add unit/regression tests and configure pytest pythonpath to import custom_components.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
custom_components/reflex_clerk_api/clerk_provider.py Adds JWT leeway + expired-token handling; rewrites session sync JS logic and imports.
tests/test_clerk_provider_unit.py Adds tests for expired-token handling and a string regression test for generated JS.
pyproject.toml Adds pytest pythonpath so tests can import custom_components.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +418 to +423
const stateKey = isSignedIn ? "signed_in" : "signed_out"
if (
lastSentRef.current?.stateKey === stateKey &&
lastSentRef.current?.addEvents === addEvents
) return
lastSentRef.current = {{ stateKey, addEvents }}
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

lastSentRef.current is updated before the async getToken() chain runs. If getToken fails/returns a falsy token and you send clear_clerk_session, the effect will not run again while isSignedIn and addEvents stay the same (deps unchanged), so the backend session can remain cleared even though Clerk is still signed in. Consider only updating the dedupe ref after successfully dispatching set_clerk_session, and/or introduce an explicit retry trigger (e.g., useState/useReducer + backoff) when token retrieval fails.

Copilot uses AI. Check for mistakes.
Comment on lines +425 to +441
if (isSignedIn) {{
// Prefer a fresh token; cached tokens can be close to expiry.
// If this Clerk version doesn't support skipCache, fall back to the default call.
Promise.resolve()
.then(() => getToken({{ skipCache: true }}))
.catch(() => getToken())
.then(token => {{
if (token) {{
addEvents([ReflexEvent("{state}.set_clerk_session", {{token}})])
}} else {{
// Token unavailable despite isSignedIn - clear to avoid stuck auth state.
addEvents([ReflexEvent("{state}.clear_clerk_session")])
}}
}}).catch(() => {{
// Token retrieval failed - clear to avoid stuck auth state.
addEvents([ReflexEvent("{state}.clear_clerk_session")])
}})
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

In the isSignedIn branch, token fetch errors (including the skipCache attempt and the fallback) immediately result in dispatching clear_clerk_session. For transient token retrieval failures this can force a backend logout even though Clerk remains signed in, and combined with the current dedupe logic it may prevent the backend from ever re-syncing. Prefer retrying token retrieval (possibly with a short delay/backoff) before clearing, or ensure the failure path triggers a subsequent re-sync attempt.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants