Skip to content
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

Always redirected to /auth/signin after updating to 3.8.0 #1467

Closed
ValentinH opened this issue Mar 7, 2021 · 7 comments · Fixed by #1468
Closed

Always redirected to /auth/signin after updating to 3.8.0 #1467

ValentinH opened this issue Mar 7, 2021 · 7 comments · Fixed by #1468
Labels
bug Something isn't working

Comments

@ValentinH
Copy link
Contributor

ValentinH commented Mar 7, 2021

Describe the bug
I noticed an issue after upgrading to today's latest and after trying each version between 3.1.0 and 3.11.0, I managed to determine that the issue mentioned below started with 3.8.0. Even though the release notes only mentioned adding the Instagram provider, I noticed that there was also quite a large refactoring of the client code in #1428. I think the issue comes from this commit.

The issue I have is that if I go to a page different than /auth/signin, I'm always redirected to /auth/signin even though I'm already logged-in. Then, once the signin page opens, I'm redirected back to the original requested page as I'm already logged-in.

Then, when I land on the original page, something extremely slow is going on and I can see my chrome devtools using more than 100% of CPU. I have the same behaviour on Firefox though. (This slowness is only on 3.8.0 and not on the latest though, probably fixed by #1462).

Steps to reproduce
My application is quite large so I'm still in the process of trying to find why I get redirected on load.

Expected behavior
When I am logged-in and refresh a page, I should not be redirected to the signin page.

Screenshots
I cannot tell if this happens before or after the redirection but I actually have an error in the console:
image

It might be because the /session call is cancelled:
image

@ValentinH ValentinH added the bug Something isn't working label Mar 7, 2021
@ValentinH
Copy link
Contributor Author

I actually found why I'm redirected to the signin page: this is coming from a part of my code that calls signIn() in a useEffect() of a page if the session returned by useSession() is undefined and the isLoading is false.

I commented the call to signIn and instead added a log for the session state and the place where I would redirect:
image

As you can see, the third log is wrong as the session is undefined while it is defined on the next session log.

ValentinH pushed a commit to ValentinH/next-auth that referenced this issue Mar 7, 2021
This is fixing nextauthjs#1467.

The issue was due to doing the `setLoading(false)` in the finally:  as we can do an early return [here](https://github.com/nextauthjs/next-auth/blob/a7e08e2a3266efa9c82eb859e7141c798fcf07ae/src/client/index.js#L100-L100), we would still go to the finally and mark the session as being loaded.

I simply removed the `finally` block to only set the `loading` state to false when:
- the data is ready
- an error occures
balazsorban44 pushed a commit that referenced this issue Mar 7, 2021
This is fixing #1467.

The issue was due to doing the `setLoading(false)` in the finally:  as we can do an early return [here](https://github.com/nextauthjs/next-auth/blob/a7e08e2a3266efa9c82eb859e7141c798fcf07ae/src/client/index.js#L100-L100), we would still go to the finally and mark the session as being loaded.

I simply removed the `finally` block to only set the `loading` state to false when:
- the data is ready
- an error occures
@github-actions
Copy link

github-actions bot commented Mar 7, 2021

🎉 This issue has been resolved in version 3.11.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@balazsorban44
Copy link
Member

Thanks for taking the time to hunt this down, and actually creating a PR for it! Yes, refactorings shouldn't be visible to users, that is why it wasn't mentioned in the release notes. Of course, if something breaks because of them, it might be harder to find the issue 😬. Glad that you did anyway! Unfortunately, we still don't have a robust test system in place to avoid problems like this, but it is being worked on.

@ValentinH
Copy link
Contributor Author

Thank you for taking over this awesome package 🙂

@balazsorban44
Copy link
Member

balazsorban44 commented Mar 7, 2021

Haven't taken over, I still have Iain involved in important decisions and he gives me infinite good advice from the background as he hasn't had a lot of free time on his hand in the recent past. Not to mention that the core logic is solely his work, I just happen to maintain it right now. Also I have other maintainers helping out from time to time. But I'm glad that you like me working on this! 😊

@ValentinH
Copy link
Contributor Author

Yes I definitely don't forget the job done by Iain. Open-source requires both time and motivation and the best is to have multiple people involved. That's why I'm happy to see you jumping in as Iain might have less time for this project 🙂

@github-actions
Copy link

🎉 This issue has been resolved in version 4.0.0-next.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment