-
Notifications
You must be signed in to change notification settings - Fork 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: redirect user if they visit onboarding page without auth #1596
fix: redirect user if they visit onboarding page without auth #1596
Conversation
…authenticated session
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Thank you for following the naming conventions for pull request titles! 🙏 |
apps/web/app/(app)/onboarding/page.tsxInstead of throwing an error when no environment is found for the user, it would be better to redirect the user to a page where they can create or select an environment. This would improve the user experience and make the application more robust. if (!environment) {
redirect("/environment/select");
} It's a good practice to avoid using the optional chaining operator (?) when you are sure that the object is not null or undefined. In this case, since you are checking if the session exists before accessing the user id, there is no need to use the optional chaining operator when accessing the user id. const userId = session.user.id; |
|
||
export default async function OnboardingPage() { | ||
const session = await getServerSession(authOptions); | ||
if (!session) { | ||
throw new Error("No session found"); | ||
redirect("/auth/login"); | ||
} | ||
const userId = session?.user.id; |
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.
Removed the optional chaining operator when accessing the user id, as it's unnecessary due to the preceding null check on the session object.
const userId = session?.user.id; | |
const userId = session.user.id; |
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.
🙏
What does this PR do?
Sentry reported 46 instances of users getting a Session not Found error specifically on the Onboarding page that we have. We currently just show the user an error without any CTA. This case is possible for when a token expires or the session JWT is not valid anymore. To tackle this, we now redirect them back to the login page so that a new token can be generated and then they will be taken to the page they should be as per the normal flow.
Type of change
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated