Skip to content

fix: propagate refresh token persistence errors#2149

Open
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/auth-refresh-save-token-errors
Open

fix: propagate refresh token persistence errors#2149
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/auth-refresh-save-token-errors

Conversation

@he-yufeng
Copy link
Copy Markdown

Summary

Fixes #2034.

When refresh-token flow succeeds but provider.saveTokens() fails, the current auth() path catches that persistence error in the same try block as refreshAuthorization(). That makes the SDK fall through to a fresh authorization redirect even though the authorization server may already have rotated the refresh token.

This separates the refresh request from token persistence. Refresh failures keep the existing fallback behavior, while persistence failures now propagate to the caller so filesystem or storage problems are visible and the client does not silently lose the newly issued tokens.

To verify

  • pnpm --filter @modelcontextprotocol/client exec vitest run test/client/auth.test.ts -t "propagates token persistence errors"
  • pnpm --filter @modelcontextprotocol/client exec vitest run test/client/auth.test.ts
  • pnpm --filter @modelcontextprotocol/client typecheck
  • pnpm --filter @modelcontextprotocol/client lint
  • git diff --check
  • pre-push hook: full workspace typecheck, build, and lint

@he-yufeng he-yufeng requested a review from a team as a code owner May 24, 2026 09:06
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 24, 2026

⚠️ No Changeset found

Latest commit: fa125f2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 24, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2149

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2149

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2149

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2149

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2149

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2149

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2149

commit: fa125f2

@he-yufeng he-yufeng force-pushed the fix/auth-refresh-save-token-errors branch from 1389aad to fa125f2 Compare May 24, 2026 09:59
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.

auth() silently swallows non-OAuthError exceptions from refreshAuthorization / saveTokens, preventing token persistence

1 participant