Skip to content

Fix spurious logouts by deduplicating concurrent token renewals#17858

Merged
FelixMalfait merged 2 commits intomainfrom
fix/deduplicate-token-renewal
Feb 11, 2026
Merged

Fix spurious logouts by deduplicating concurrent token renewals#17858
FelixMalfait merged 2 commits intomainfrom
fix/deduplicate-token-renewal

Conversation

@FelixMalfait
Copy link
Copy Markdown
Member

Summary

  • When returning to the app after idle (or after a deploy), the expired access token causes multiple simultaneous GraphQL queries to fail with UNAUTHENTICATED. Previously, each failure independently triggered its own renewToken call with the same refresh token. If any single renewal failed (e.g. server briefly slow after a deploy), the catch handler would nuke the session and redirect to sign-in — even if another concurrent renewal had already succeeded and written valid tokens.
  • This adds a shared renewalPromise so that only the first UNAUTHENTICATED error triggers a server-side renewal. All concurrent callers await the same promise and replay their operations once it resolves. This eliminates redundant refresh token rotation on the server and removes the race condition where a straggling failure could log out an already-renewed session.

Test plan

  • Log in, wait >30 minutes (or manually expire the access token), then interact with the app — should silently renew without redirect to sign-in
  • Open browser DevTools Network tab, trigger the above scenario, and verify only one renewToken mutation is sent (instead of N)
  • With server temporarily stopped, verify that a genuine renewal failure still correctly redirects to sign-in (single onUnauthenticatedError call)
  • Open multiple browser tabs, let access tokens expire, interact in one tab — other tabs should also recover gracefully on their next request

Made with Cursor

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 11, 2026

Greptile Overview

Greptile Summary

Adds token renewal deduplication to prevent multiple concurrent GraphQL queries from each triggering their own renewToken call when the access token expires. Uses a shared renewalPromise so only the first UNAUTHENTICATED error initiates renewal, while concurrent requests await the same promise.

Key Changes:

  • Introduced module-scoped renewalPromise variable to track ongoing renewal
  • Modified handleTokenRenewal to check if renewal is in progress before creating a new promise
  • All concurrent callers now await the same renewal operation and replay their requests once complete

Issue Found:

  • The .catch() handler in the renewal promise consumes errors without re-throwing, which means concurrent callers awaiting the shared promise won't detect renewal failures and may incorrectly proceed with their operations

Confidence Score: 3/5

  • This PR fixes a legitimate race condition but introduces a critical error handling bug that could cause failed renewals to be ignored by concurrent requests
  • The deduplication logic is sound and addresses the described race condition, but the error handling flaw in the .catch() block is critical - it prevents concurrent callers from detecting renewal failures, potentially leaving them retrying operations with expired tokens
  • The error handling in apollo.factory.ts at lines 162-168 needs immediate attention to re-throw errors for concurrent callers

Important Files Changed

Filename Overview
packages/twenty-front/src/modules/apollo/services/apollo.factory.ts Implements token renewal deduplication to prevent race conditions, but has a critical error handling flaw where concurrent callers won't properly handle renewal failures

Sequence Diagram

sequenceDiagram
    participant C1 as Concurrent Request 1
    participant C2 as Concurrent Request 2
    participant C3 as Concurrent Request 3
    participant Handler as handleTokenRenewal
    participant Server as Auth Server
    
    Note over C1,C3: Access tokens expired, all fail with UNAUTHENTICATED
    
    C1->>Handler: UNAUTHENTICATED error
    activate Handler
    Note over Handler: renewalPromise = null, create new promise
    Handler->>Server: renewToken(refreshToken)
    activate Server
    
    C2->>Handler: UNAUTHENTICATED error
    Note over Handler: renewalPromise exists, reuse same promise
    
    C3->>Handler: UNAUTHENTICATED error
    Note over Handler: renewalPromise exists, reuse same promise
    
    Server-->>Handler: Return new tokens
    deactivate Server
    Handler->>Handler: Update tokens in cookie/state
    Handler->>Handler: renewalPromise = null (finally)
    deactivate Handler
    
    Handler-->>C1: forward(operation) with new tokens
    Handler-->>C2: forward(operation) with new tokens
    Handler-->>C3: forward(operation) with new tokens
    
    Note over C1,C3: All requests retry successfully with refreshed tokens
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 11, 2026

Additional Comments (1)

packages/twenty-front/src/modules/apollo/services/apollo.factory.ts
The .catch() handler consumes the error, preventing it from propagating to concurrent callers. When renewal fails, the first caller triggers onUnauthenticatedError?.(), but subsequent callers awaiting the same renewalPromise via fromPromise(renewalPromise) won't see the error and will incorrectly proceed with forward(operation) as if the renewal succeeded.

            .catch((error) => {
              // eslint-disable-next-line no-console
              console.log(
                'Failed to renew token, triggering unauthenticated error from handleTokenRenewal',
              );
              onUnauthenticatedError?.();
              // Re-throw so concurrent callers also see the failure
              throw error;
            })

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 11, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:58501

This environment will automatically shut down when the PR is closed or after 5 hours.

When returning to the app after idle (expired access token), multiple
GraphQL queries fail with UNAUTHENTICATED simultaneously. Each one
independently triggered a renewToken call with the same refresh token.
If any single renewal failed (e.g. server briefly slow after a deploy),
the catch handler would clear the session and redirect to sign-in, even
if another concurrent renewal had already succeeded.

This was compounded by having two separate ApolloFactory instances
(/graphql and /metadata), each racing to renew the same refresh token.
The /metadata client's renewal could also fail because the RenewToken
mutation may not be exposed on that endpoint.

Fix: lift renewalPromise to module scope so all ApolloFactory instances
share a single in-flight renewal, and always send the RenewToken
mutation to /graphql.

Co-authored-by: Cursor <cursoragent@cursor.com>
@FelixMalfait FelixMalfait force-pushed the fix/deduplicate-token-renewal branch from 9107511 to bc2099b Compare February 11, 2026 12:01
Copy link
Copy Markdown
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

@FelixMalfait FelixMalfait added this pull request to the merge queue Feb 11, 2026
Merged via the queue into main with commit 2237273 Feb 11, 2026
72 checks passed
@FelixMalfait FelixMalfait deleted the fix/deduplicate-token-renewal branch February 11, 2026 12:44
@twenty-eng-sync
Copy link
Copy Markdown

Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

1 similar comment
@twenty-eng-sync
Copy link
Copy Markdown

Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

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.

3 participants