-
Notifications
You must be signed in to change notification settings - Fork 537
fix: trial not starting during onboarding #3805
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
base: main
Are you sure you want to change the base?
Conversation
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }; | ||
|
|
||
| void handle(); | ||
| }, [auth, store]); | ||
|
|
||
| return () => { | ||
| abortController.abort(); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 AbortController cleanup + hasHandledRef causes infinite loading in React StrictMode
In development mode with React StrictMode (enabled at apps/desktop/src/main.tsx:133), the component gets stuck on the loading screen forever.
Root Cause: StrictMode double-mount interacts badly with hasHandledRef + AbortController
React StrictMode unmounts and re-mounts components to surface side-effect bugs. The sequence is:
- First mount: Effect runs →
hasHandledRef.currentset totrue→handle()starts async → cleanup function returned - StrictMode unmount: Cleanup runs →
abortController.abort()→ the in-flighthandle()will see the abort - Second mount: Effect runs again →
hasHandledRef.currentis stilltrue(refs persist across StrictMode remounts) → returns early without starting any work or returning a cleanup function
The handle() from step 1 reaches pollForTrialActivation which detects the aborted signal and returns { status: "aborted" }. Back in handle() at line 66, if (result.status === "aborted") return; causes it to return without calling setIsLoading(false). The second mount never starts a new handle() because hasHandledRef blocks it.
Result: isLoading remains true forever, and the user sees the spinner indefinitely.
Impact: Developers working on the onboarding flow in dev mode will be unable to proceed past the final step. This doesn't affect production builds since StrictMode effects only double-fire in development.
The fix should either: (a) remove hasHandledRef and rely solely on the AbortController cleanup (letting StrictMode re-run the effect properly on remount), or (b) reset hasHandledRef.current = false in the cleanup function so the second mount can re-execute.
(Refers to lines 38-83)
Prompt for agents
The hasHandledRef guard and AbortController cleanup are incompatible under React StrictMode. In StrictMode, the cleanup aborts the first execution, but hasHandledRef prevents the second execution from starting, leaving the user stuck on loading.
Two possible fixes:
Option A (recommended): Remove hasHandledRef entirely and rely on AbortController for cleanup. The effect will re-run on StrictMode remount, starting a fresh handle() with a new AbortController. The previous one is properly cleaned up via abort.
In apps/desktop/src/components/onboarding/final.tsx, remove lines 29 (hasHandledRef declaration), 38-40 (the guard and set), and keep the rest of the useEffect as-is. The AbortController cleanup already handles the double-mount case correctly.
Option B: Reset hasHandledRef in the cleanup function by adding hasHandledRef.current = false before abortController.abort() in the cleanup at line 80-82.
Was this helpful? React with 👍 or 👎 to provide feedback.
fix: trial not starting during onboarding
Summary
Fixes trial not starting for new users during the onboarding flow. Two bugs in
final.tsx:Stale closure bug: The
useEffecthad[auth, store]as dependencies. Whenauth.refreshSession()updated the session,authobject identity changed, causing React to re-fire the effect — buthasHandledRefblocked re-execution. The asynchandle()continued running with a staleauthfrom the original closure. Fixed by using refs (authRef,storeRef) and changing deps to[].Fixed 3s sleep was unreliable: The Stripe subscription creation → webhook → Supabase auth hook → JWT claims pipeline takes variable time. Replaced
sleep(3000)with exponential backoff polling (1s → 5s cap, 10 attempts) that checks JWT claims for thehyprnote_proentitlement before proceeding.Also adds proper
AbortControllercleanup so the async polling is cancelled if the component unmounts.Review & Testing Checklist for Human
authCommands.decodeClaimsworks correctly with the access token format — the polling utility depends on this to detecthyprnote_proin entitlements.skipLoginpath still works — whenauth.sessionis null, the effect should setisLoading=falseimmediately without attempting trial.Notes
devin/1770469282-exponential-backoff-trial-pollingthat was never merged. This PR applies equivalent fixes cleanly on top of currentmain.