-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix origin migrator for SSO logins #10920
Conversation
For some reason this was trying to close the same window twice when the app was reloaded after an SSO login. Possibly also a problem on electron < 6 - presumably a race condition.
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.
Hmm, why would the events come back multiple times...? Seems suspicious right?
Well, the events coming back is legit since with SSO we are coming back to the page and therefore loading up the app again, causing another origin migration attempt. In this case the old listeners will still be there. |
Hmm, I see... What about the case of getting Effectively I am wondering if we need to remove all listeners whenever any one of them fires. |
Yeah, we technically do, although in practice nobody should actually need the migrator now so there's practically zero chance of actually hitting that case. That said, I guess if we're fixing it we should fix it properly. |
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.
Thanks, looks good! 😁
For some reason this was trying to close the same window twice
when the app was reloaded after an SSO login. Possibly also a
problem on electron < 6 - presumably a race condition.
Merging to release branch