Skip to content

fix(login): move try/catch inside startTransition and use notificationManager#164

Merged
farhan2742 merged 1 commit into
arbisoft:devfrom
arbisoft-qaisarirfan:ASP-161
Jun 23, 2026
Merged

fix(login): move try/catch inside startTransition and use notificationManager#164
farhan2742 merged 1 commit into
arbisoft:devfrom
arbisoft-qaisarirfan:ASP-161

Conversation

@arbisoft-qaisarirfan

Copy link
Copy Markdown
Contributor

🚀 Description

Fix error handling in the Google login flow where the try/catch was placed outside startTransition, causing async errors thrown inside the transition callback to be silently swallowed rather than caught.

📌 Summary

The try/catch block was wrapping startTransition(...) itself, but since startTransition schedules an async callback, any error thrown inside it would escape the outer catch. This PR moves try/catch inside the startTransition callback to correctly catch login errors, and uses notificationManager (the imperative API) instead of the showNotification hook — which is not reliably callable inside async transition callbacks.

🔧 Changes Implemented

  • ✅ Moved try/catch inside the startTransition callback to properly catch async errors
  • ✅ Replaced showNotification (hook) with notificationManager.showNotification (imperative API) for error display inside the async transition
  • ✅ Updated import to include notificationManager from @/components/Notification
  • ✅ Simplified error message to "Something went wrong."

🛠️ How It Works?

  1. When Google returns a credential, startTransition schedules the async login flow.
  2. If loginAndSetCookie or dispatch throws, the catch block inside the transition now correctly intercepts the error.
  3. notificationManager.showNotification is called imperatively (no React hook constraints) to display the error toast to the user.

✅ Checklist Before Merging

  • Tested Google login with a valid account — redirects correctly.
  • Tested Google login failure scenario — error notification appears.
  • Verified no regression in the existing showNotification usage for the "No credential received" case.
  • Ensured notification displays correctly in both light and dark themes.

🔗 Related Issues

https://projects.arbisoft.com/arbisoft/browse/ASP-161/

📢 Notes for Reviewers

  • The key fix is the placement of try/catch — previously it wrapped startTransition(callback) which does not propagate async errors thrown inside the callback. Moving it inside is the correct pattern.
  • notificationManager is the imperative alternative to the useNotification hook; it's the right choice when calling from inside a non-hook context like an async transition callback.

@farhan2742 farhan2742 merged commit 2aee0ac into arbisoft:dev Jun 23, 2026
1 check passed
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.

2 participants