Add token renewal to agent chat transport#15727
Conversation
- Added `useSetRecoilState` for managing token state. - Implemented `retryFetchWithRenewedToken` function to handle token renewal and retry fetch requests. - Updated chat transport to utilize the new token renewal logic when a 401 status is encountered.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| updatedHeaders.set('Authorization', `Bearer ${renewedAccessToken}`); | ||
|
|
||
| return fetch(input, { | ||
| ...init, | ||
| headers: updatedHeaders, | ||
| }); | ||
| } catch { | ||
| setTokenPair(null); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Bug: retryFetchWithRenewedToken lacks synchronization, causing concurrent renewToken() calls and redundant server requests during multiple 401s.
Severity: HIGH | Confidence: 1.00
🔍 Detailed Analysis
When multiple HTTP requests simultaneously receive a 401 response, the retryFetchWithRenewedToken function in useAgentChat.ts allows each request to independently call renewToken(). This lack of synchronization leads to multiple concurrent token renewal requests being sent to the server, resulting in redundant API calls and concurrent updates to tokenPair and cookieStorage instead of a single, serialized renewal process.
💡 Suggested Fix
Implement a synchronization mechanism within retryFetchWithRenewedToken, similar to the fromPromise pattern used in apollo.factory.ts, to ensure only one renewToken() call executes at a time.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/twenty-front/src/modules/ai/hooks/useAgentChat.ts#L54-L97
Potential issue: When multiple HTTP requests simultaneously receive a 401 response, the
`retryFetchWithRenewedToken` function in `useAgentChat.ts` allows each request to
independently call `renewToken()`. This lack of synchronization leads to multiple
concurrent token renewal requests being sent to the server, resulting in redundant API
calls and concurrent updates to `tokenPair` and `cookieStorage` instead of a single,
serialized renewal process.
Did we get this right? 👍 / 👎 to inform future reviews.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds automatic token renewal to the agent chat transport layer to handle 401 authentication errors gracefully.
Key Changes:
- Implemented custom fetch wrapper in
DefaultChatTransportthat intercepts 401 responses - Added
retryFetchWithRenewedTokenfunction to handle token renewal and retry logic - Imported necessary dependencies:
renewTokenfromAuthService,tokenPairState, and cookie storage utilities - Follows the same token renewal pattern used in
apollo.factory.tsfor consistency
Implementation Details:
The solution intercepts HTTP responses at the transport level. When a 401 is detected, it calls the existing renewToken service, updates both Recoil state and cookie storage with the new tokens, then retries the original request with the refreshed authentication token. If renewal fails, it clears the token state and returns the original 401 response.
Confidence Score: 4/5
- This PR is safe to merge with minor style improvements recommended
- The implementation follows existing patterns from
apollo.factory.tsand correctly handles the token renewal flow. The logic is sound - it intercepts 401s, renews tokens, updates state, and retries. However, there are two minor issues: (1) the empty catch block silently swallows errors which could make debugging harder, and (2) one comment I left about infinite loops was incorrect upon deeper analysis - the fetch inretryFetchWithRenewedTokenuses the global fetch, not the wrapped one, so no recursion occurs. The core functionality is solid. - No files require special attention - the implementation is straightforward and follows established patterns
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-front/src/modules/ai/hooks/useAgentChat.ts | 4/5 | Added token renewal mechanism for 401 responses in agent chat transport with proper error handling |
Sequence Diagram
sequenceDiagram
participant User
participant AgentChat as useAgentChat Hook
participant Transport as DefaultChatTransport
participant API as Agent Chat API
participant Auth as renewToken Service
participant Storage as Cookie Storage
User->>AgentChat: Send message
AgentChat->>Transport: sendMessage()
Transport->>Transport: Custom fetch wrapper
Transport->>API: POST agent chat stream endpoint
alt Valid Token
API-->>Transport: HTTP 200 Stream response
Transport-->>AgentChat: messages updated
AgentChat-->>User: Display response
else Expired Token
API-->>Transport: HTTP 401 Response
Transport->>Transport: retryFetchWithRenewedToken()
Transport->>Auth: Call renewToken with current tokenPair
Auth->>API: GraphQL mutation to renew tokens
alt Token Renewal Success
API-->>Auth: New access and refresh tokens
Auth-->>Transport: Returns renewed tokens
Transport->>Storage: Save tokens to cookies
Transport->>AgentChat: Update token state
Transport->>API: Retry POST with refreshed token
API-->>Transport: HTTP 200 Stream response
Transport-->>AgentChat: messages updated
AgentChat-->>User: Display response
else Token Renewal Failure
API-->>Auth: Error or invalid response
Auth-->>Transport: Returns null or throws error
Transport->>Storage: Clear stored tokens
Transport->>AgentChat: Set token state to null
Transport-->>AgentChat: Return original 401 response
AgentChat-->>User: Show error state
end
end
1 file reviewed, 2 comments
| ...init, | ||
| headers: updatedHeaders, | ||
| }); | ||
| } catch { |
There was a problem hiding this comment.
style: empty catch block silently ignores all errors during token renewal, including network failures and unexpected errors
| } catch { | |
| } catch (error) { | |
| if (isDebugMode) { | |
| logDebug(`Token renewal failed: ${error}`); | |
| } | |
| setTokenPair(null); | |
| return null; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/ai/hooks/useAgentChat.ts
Line: 93:93
Comment:
**style:** empty catch block silently ignores all errors during token renewal, including network failures and unexpected errors
```suggestion
} catch (error) {
if (isDebugMode) {
logDebug(`Token renewal failed: ${error}`);
}
setTokenPair(null);
return null;
}
```
How can I resolve this? If you propose a fix, please make it concise.| fetch: async (input, init) => { | ||
| const response = await fetch(input, init); | ||
|
|
||
| if (response.status !== 401) { | ||
| return response; | ||
| } | ||
|
|
||
| const retriedResponse = await retryFetchWithRenewedToken(input, init); | ||
|
|
||
| return retriedResponse ?? response; | ||
| }, |
There was a problem hiding this comment.
logic: potential infinite loop - if token renewal returns 401 (e.g. invalid refresh token), this will retry indefinitely without preventing another 401 response
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/ai/hooks/useAgentChat.ts
Line: 105:115
Comment:
**logic:** potential infinite loop - if token renewal returns 401 (e.g. invalid refresh token), this will retry indefinitely without preventing another 401 response
How can I resolve this? If you propose a fix, please make it concise.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:52108 This environment will automatically shut down when the PR is closed or after 5 hours. |
|
Thanks @abdulrahmancodes for your contribution! |
2 participants
No description provided.