-
Notifications
You must be signed in to change notification settings - Fork 142
fix: set cookie date to string conversion #571
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change updates the way the session cookie’s expiration date is formatted in the server response. Instead of appending the raw value of Changes
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ff78045
to
45bf701
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/server/config/trpc.ts (1)
236-236
: Good fix for the date conversion issue, but consider usingtoUTCString()
instead.Converting the date using
toISOString()
is a good improvement over implicit conversion, as it ensures consistent behavior across different environments. This explains why the issue was happening in local mode but not in the demo environment.However, for HTTP cookies, the
Expires
attribute should conform to RFC 7231, which uses a different format than ISO 8601. Consider usingtoUTCString()
instead, which outputs the format required by the HTTP specification:-`${AUTH_COOKIE_NAME}=${token}; Path=/; Expires=${session.expiresAt.toISOString()}; SameSite=Lax; HttpOnly; Secure=${env.NODE_ENV === 'production'}` +`${AUTH_COOKIE_NAME}=${token}; Path=/; Expires=${session.expiresAt.toUTCString()}; SameSite=Lax; HttpOnly; Secure=${env.NODE_ENV === 'production'}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/config/trpc.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/server/config/trpc.ts (1)
src/server/config/session.ts (1)
AUTH_COOKIE_NAME
(15-15)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright E2E Tests
|
Describe your changes
When working in local mode, the conversion of the session.expiresAt (a Date) value to a string when appending the Set-Cookie to the response headers caused an error. This would cause a fail any call to a route with a protectedProcedure.
Odly enough this is not a problem we see on the demo.
We suppose it is because vercel uses a different runtime (edge) on their servers.
Screenshots
Documentation
Checklist
pnpm storybook
command and everything is workingSummary by CodeRabbit