Skip to content

fix(clerk-js): prevent broadcast failure from evicting a cached session token#8969

Open
jacekradko wants to merge 1 commit into
mainfrom
jacek/sdk-119-broadcast-postmessage-failure-evicts-a-valid-cached-session
Open

fix(clerk-js): prevent broadcast failure from evicting a cached session token#8969
jacekradko wants to merge 1 commit into
mainfrom
jacek/sdk-119-broadcast-postmessage-failure-evicts-a-valid-cached-session

Conversation

@jacekradko

@jacekradko jacekradko commented Jun 23, 2026

Copy link
Copy Markdown
Member

SessionTokenCache.setInternal runs the cross-tab broadcast as the last step inside the token-resolution .then, and that whole chain ends in .catch(() => deleteKey()). So if postMessage (or getRawString) throws, the failure lands in that catch and evicts the token that was just cached and timer-scheduled, turning the next getToken() into an avoidable network fetch.

This isolates the broadcast side-effect in its own try/catch (logs a warning, keeps the cached token). The regression test reproduces the bug: with the guard removed, size() drops to 0 after a throwing postMessage; with the guard the token stays cached.

Fixes SDK-119. It is pre-existing in shipped clerk-js and a backport candidate, so it is scoped to just the broadcast guard, independent of the TokenCache decouple in #8860.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where cross-tab token broadcasting failures would unnecessarily evict cached session tokens, preventing unexpected follow-up network requests on token retrieval.
  • Tests

    • Added test coverage for broadcast resilience to ensure cached tokens remain intact when cross-tab synchronization encounters message delivery failures.

@changeset-bot

changeset-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 96b8c0f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/electron Patch
@clerk/expo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Jun 23, 2026 12:55pm
swingset Ready Ready Preview, Comment Jun 23, 2026 12:55pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 1c26dfeb-ad96-48b1-b4f9-96eaa6b73852

📥 Commits

Reviewing files that changed from the base of the PR and between 52d310c and 96b8c0f.

📒 Files selected for processing (3)
  • .changeset/tokencache-broadcast-eviction.md
  • packages/clerk-js/src/core/__tests__/tokenCache.test.ts
  • packages/clerk-js/src/core/tokenCache.ts

📝 Walkthrough

Walkthrough

setInternal in tokenCache.ts now wraps the cross-tab BroadcastChannel.postMessage call in a try/catch. Broadcast failures log a warning instead of propagating to the outer .catch(() => deleteKey()), preserving the cached token. A new broadcast resilience test and changeset entry document the fix.

Changes

BroadcastChannel failure isolation in token cache

Layer / File(s) Summary
Broadcast try/catch fix, resilience test, and changeset
packages/clerk-js/src/core/tokenCache.ts, packages/clerk-js/src/core/__tests__/tokenCache.test.ts, .changeset/tokencache-broadcast-eviction.md
setInternal's resolved-token broadcast is wrapped in try/catch; errors log a warning and skip the outer .catch(() => deleteKey()) path. A new broadcast resilience test confirms the cache entry survives a postMessage throw. The changeset records the patch release.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 The broadcast may fail, the message may miss,
But the token stays safe — nothing's amiss!
A catch wraps the error, a warning gets logged,
No eviction occurs, no session is robbed.
Hop hop, little cache, you're resilient now! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main fix: preventing broadcast failures from causing token cache eviction.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Jun 23, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@8969

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@8969

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@8969

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@8969

@clerk/electron

npm i https://pkg.pr.new/@clerk/electron@8969

@clerk/electron-passkeys

npm i https://pkg.pr.new/@clerk/electron-passkeys@8969

@clerk/eslint-plugin

npm i https://pkg.pr.new/@clerk/eslint-plugin@8969

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@8969

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@8969

@clerk/express

npm i https://pkg.pr.new/@clerk/express@8969

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@8969

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@8969

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@8969

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@8969

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@8969

@clerk/react

npm i https://pkg.pr.new/@clerk/react@8969

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@8969

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@8969

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@8969

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@8969

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@8969

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@8969

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@8969

commit: 96b8c0f

@github-actions

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-06-23T12:56:38.801Z

Summary

Metric Count
Packages analyzed 19
Packages with changes 0
🔴 Breaking changes 0
🟡 Non-breaking changes 0
🟢 Additions 0

No API Changes Detected

All packages have stable APIs with no detected changes.


Report generated by Break Check

Last ran on 96b8c0f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant