-
Notifications
You must be signed in to change notification settings - Fork 442
Fix session cookie creation race: skip initial token refresh, wrap extension auth hooks #6563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
3719a46
93c3c3f
b4cfaf7
fd75975
17f2048
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…ener lifecycle; address PR #6563 review comments
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,11 @@ export const useExtensionService = () => { | |
| const extensionStore = useExtensionStore() | ||
| const settingStore = useSettingStore() | ||
| const keybindingStore = useKeybindingStore() | ||
| const { wrapWithErrorHandling, wrapWithErrorHandlingAsync } = | ||
| useErrorHandling() | ||
| const { | ||
| wrapWithErrorHandling, | ||
| wrapWithErrorHandlingAsync, | ||
| toastErrorHandler | ||
| } = useErrorHandling() | ||
|
|
||
| /** | ||
| * Loads all extensions from the API into the window in parallel | ||
|
|
@@ -80,7 +83,15 @@ export const useExtensionService = () => { | |
| if (extension.onAuthUserResolved) { | ||
| const { onUserResolved } = useCurrentUser() | ||
| const handleUserResolved = wrapWithErrorHandlingAsync( | ||
| (user: AuthUserInfo) => extension.onAuthUserResolved?.(user, app) | ||
| (user: AuthUserInfo) => extension.onAuthUserResolved?.(user, app), | ||
| (error) => { | ||
| console.error('[Extension Auth Hook Error]', { | ||
| extension: extension.name, | ||
| hook: 'onAuthUserResolved', | ||
| error | ||
| }) | ||
| toastErrorHandler(error) | ||
| } | ||
| ) | ||
| onUserResolved((user) => { | ||
| void handleUserResolved(user) | ||
benceruleanlu marked this conversation as resolved.
Show resolved
Hide resolved
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed: the async wrapper now adds structured error context for auth hooks — we log the extension name and the hook (e.g., |
||
|
|
@@ -89,8 +100,16 @@ export const useExtensionService = () => { | |
|
|
||
| if (extension.onAuthTokenRefreshed) { | ||
| const { onTokenRefreshed } = useCurrentUser() | ||
| const handleTokenRefreshed = wrapWithErrorHandlingAsync(() => | ||
| extension.onAuthTokenRefreshed?.() | ||
| const handleTokenRefreshed = wrapWithErrorHandlingAsync( | ||
| () => extension.onAuthTokenRefreshed?.(), | ||
| (error) => { | ||
| console.error('[Extension Auth Hook Error]', { | ||
| extension: extension.name, | ||
| hook: 'onAuthTokenRefreshed', | ||
| error | ||
| }) | ||
| toastErrorHandler(error) | ||
| } | ||
| ) | ||
| onTokenRefreshed(() => { | ||
| void handleTokenRefreshed() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarification: the wrapper’s return is intentionally discarded ( |
||
|
|
@@ -99,8 +118,16 @@ export const useExtensionService = () => { | |
|
|
||
| if (extension.onAuthUserLogout) { | ||
| const { onUserLogout } = useCurrentUser() | ||
| const handleUserLogout = wrapWithErrorHandlingAsync(() => | ||
| extension.onAuthUserLogout?.() | ||
| const handleUserLogout = wrapWithErrorHandlingAsync( | ||
| () => extension.onAuthUserLogout?.(), | ||
| (error) => { | ||
| console.error('[Extension Auth Hook Error]', { | ||
| extension: extension.name, | ||
| hook: 'onAuthUserLogout', | ||
| error | ||
| }) | ||
| toastErrorHandler(error) | ||
| } | ||
| ) | ||
| onUserLogout(() => { | ||
| void handleUserLogout() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.