Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Social logins are laggy #63

Open
NBMSacha opened this issue Apr 18, 2023 · 12 comments · Fixed by #65
Open

Social logins are laggy #63

NBMSacha opened this issue Apr 18, 2023 · 12 comments · Fixed by #65
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@NBMSacha
Copy link

Describe the bug
Randomly, 70-90% of the time, social login won't be working - in production at mobula.fi for instance.

To Reproduce
Steps to reproduce the behavior:

  1. Go to mobula.fi
  2. Click on connect with Social
  3. Connect
  4. See error

Expected behavior
Successfully connect using socials.

Additional context
Been discussing with magic.link Customer Success team, they also build an MVP to reproduce the error, I'll link it down here asap.

@NBMSacha
Copy link
Author

@kesar
Copy link
Member

kesar commented May 1, 2023

I tried several times with the magiclabs sandbox and it was workings fine. Its also working fine in our project and we haven't got any other project with same problem, so I think it could be related to your general implementation rather than a lib problem.

I noticed your login dialog is a bit laggy and saw 213 warnings of your state library, so perhaps there is an infinite re-rendering loop somewhere in your code

@NBMSacha
Copy link
Author

NBMSacha commented May 1, 2023

Thanks a lot digging into it!

Magic.link team did confirm they were getting the same issue as me with the sample codesandbox example - just tried again myself, and it's still not working on my end either.

Thanks for spotting those 213 warnings.

In prod, related to magic.link I get

p.chunk~store.c74d5f2b34f2f35f8cd2.js:1 could not authenticate via cookie Error: Unable to refresh user session. Please log in using the magic link flow to enable session persistence.
    at N (app.chunk~fad58de7366495db4650cfefac2fcd61.3e9f1eaf1408d98d7487.js:1:686153)
    at Object.errorTransform (app.chunk~services.4ba9f49bdace82c06041.js:1:13777)
    at app.chunk~fad58de7366495db4650cfefac2fcd61.3e9f1eaf1408d98d7487.js:1:668722

as well as

p.chunk~vendor~fad58de7366495db4650cfefac2fcd61.27faa64f27076eec30a0.js:2 GET https://auth.magic.link/v1/session/refresh 400

Which are exactly the same issues I'm experiencing on your frontend.
image

Are you sure it's working on your app? Because when I'm trying iq.wiki, it's not working at all on my end.

@kesar
Copy link
Member

kesar commented May 1, 2023

oh, I tried email and it worked fine, but I just tried google and its not working. going to review this 👍🏻

@NBMSacha
Copy link
Author

NBMSacha commented May 1, 2023

Thanks! As I told you, happy to put some dev resources from our end into it if it can help. We've been using your code for free for months, so happy to contribute a bit if it can make this fix happen sooner 😁

@NBMSacha
Copy link
Author

NBMSacha commented May 1, 2023

Just need some guidelines - if you wish some help.

@Royal-lobster
Copy link
Member

Royal-lobster commented May 3, 2023

Made some progress yesterday, here are some insights

Found out that we are not handling the oauth state correctly, tho it used to work in previous versions, we lost the flow at some point. After fixing some code, we are left with cookie error warning -

image

Left the issue on magic discord so we can get some help about it. also, went further to get the state back after redirect from OAuth, following magic docs, fetching the data getting around 7s which is very slow, and with out it we are not getting the user state to pass back to wagmi. And last issue is with wagmi, i am still trying to figure out how we can send the user account to wagmi from our wallet constructor,

CleanShot 2023-05-03 at 15 20 12@2x

Even though the user defines autoconnect to true,

image

The connection is not happening after OAuth redirect which is leaving us with this behavior -

CleanShot.2023-05-03.at.12.07.52.mp4

Unless user clicks on connect again, it won't automatically connect after redirect.

Resources

@ZYJLiu
Copy link
Contributor

ZYJLiu commented May 9, 2023

@Royal-lobster

It seems like the issue is due to this isAuthorized check in wagmi's autoconnect.

Updating isAuthorized for the magic auth connector like this seems to resolve the issue:

  async isAuthorized() {
    try {
      const isLoggedIn = await this.magic.user.isLoggedIn();

      if (isLoggedIn) {
        return true;
      }

      const result = await this.magic.oauth.getRedirectResult();

      return result !== null;
    } catch {
      return false;
    }
  }

I combined magicConnector.ts and magicAuthConnector.ts into a single file when testing locally, so you may need to make some minor edits to work with current version of connector.

@Royal-lobster
Copy link
Member

@ZYJLiu Thanks a lot, Tried this approach which is working ! but i am getting the same issue of wagmi not connecting automatically after the flow where i had to click on connect button manually. (same as video i posted previously)

CleanShot.2023-05-03.at.12.07.52.mp4

Does it happen for you ?

@ZYJLiu
Copy link
Contributor

ZYJLiu commented May 9, 2023

This is behavior I'm seeing now after updating isAuthorized, its slow but I believe that's due to waiting for magic.oauth.getRedirectResult() like you mentioned

magic-auth

@Royal-lobster
Copy link
Member

Royal-lobster commented May 9, 2023

Wow thats true, its connecting after some time.

Weird that it takes that much time and also don't trigger loading state on Wagmi. but for now we will merge for new release, and i hope we can figure out a solution for the delay on connection issue in upcoming PRs.

@Royal-lobster
Copy link
Member

Partial fix released under @everipedia/[email protected]

@Royal-lobster Royal-lobster added bug Something isn't working help wanted Extra attention is needed labels May 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants