Fix: Define cookie priority for order placed loader#1567
Fix: Define cookie priority for order placed loader#1567vitoUwu wants to merge 1 commit intodeco-cx:mainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughA helper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vtex/loaders/orders/orderplaced.ts`:
- Around line 42-43: The resolveOrderCookie function currently returns an empty
string when no eligible cookies are found, causing an empty "cookie" header to
be forwarded; change resolveOrderCookie to return undefined instead of "" when
no cookies are present, and update the caller that forwards the header (the code
that sets the "cookie" request/header using the return of resolveOrderCookie) to
only add the "cookie" header when the value is !== undefined (i.e., guard the
header assignment so it is omitted entirely when resolveOrderCookie returns
undefined).
- Around line 32-34: The current check returns checkout-cookie mode when any
cookie exists; change the condition in the orderplaced loader to require both
cookies (specifically ensure CheckoutDataAccess and Vtex_CHKO_Auth are present
on checkoutCookies) before returning stringify(checkoutCookies); otherwise fall
through to the auth fallback path. Locate the block that uses checkoutCookies
and replace the Object.keys(checkoutCookies).length > 0 check with an explicit
presence check for checkoutCookies.CheckoutDataAccess &&
checkoutCookies.Vtex_CHKO_Auth so partial cookie sets no longer short-circuit
authentication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0445be84-a8e0-4ff1-aa0a-d80905409057
📒 Files selected for processing (1)
vtex/loaders/orders/orderplaced.ts
| if (Object.keys(checkoutCookies).length > 0) { | ||
| return stringify(checkoutCookies); | ||
| } |
There was a problem hiding this comment.
Require both checkout cookies before choosing checkout-cookie mode.
At Line 32, the condition > 0 treats a partial set as valid. If only one of CheckoutDataAccess or Vtex_CHKO_Auth is present, the loader skips auth fallback and can still fail access.
Suggested fix
- if (Object.keys(checkoutCookies).length > 0) {
+ const hasBothCheckoutCookies =
+ checkoutCookies[CHECKOUT_DATA_ACCESS_COOKIE] != null &&
+ checkoutCookies[VTEX_CHKO_AUTH] != null;
+
+ if (hasBothCheckoutCookies) {
return stringify(checkoutCookies);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Object.keys(checkoutCookies).length > 0) { | |
| return stringify(checkoutCookies); | |
| } | |
| const hasBothCheckoutCookies = | |
| checkoutCookies[CHECKOUT_DATA_ACCESS_COOKIE] != null && | |
| checkoutCookies[VTEX_CHKO_AUTH] != null; | |
| if (hasBothCheckoutCookies) { | |
| return stringify(checkoutCookies); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/loaders/orders/orderplaced.ts` around lines 32 - 34, The current check
returns checkout-cookie mode when any cookie exists; change the condition in the
orderplaced loader to require both cookies (specifically ensure
CheckoutDataAccess and Vtex_CHKO_Auth are present on checkoutCookies) before
returning stringify(checkoutCookies); otherwise fall through to the auth
fallback path. Locate the block that uses checkoutCookies and replace the
Object.keys(checkoutCookies).length > 0 check with an explicit presence check
for checkoutCookies.CheckoutDataAccess && checkoutCookies.Vtex_CHKO_Auth so
partial cookie sets no longer short-circuit authentication.
| return stringify(authCookies); | ||
| } |
There was a problem hiding this comment.
Avoid sending an empty cookie header when no eligible cookies exist.
If neither checkout nor auth cookies are found, resolveOrderCookie returns "" and Line 55 still forwards it. Prefer returning undefined and omitting the header entirely.
Suggested fix
-function resolveOrderCookie(headers: Headers): string {
+function resolveOrderCookie(headers: Headers): string | undefined {
const all = getCookies(headers);
@@
- return stringify(authCookies);
+ return Object.keys(authCookies).length > 0 ? stringify(authCookies) : undefined;
}
@@
- const cookie = resolveOrderCookie(req.headers);
+ const cookie = resolveOrderCookie(req.headers);
+ const headers = cookie ? { cookie } : undefined;
@@
- }, {
- headers: { cookie },
- }).then((res) => res.json());
+ }, {
+ headers,
+ }).then((res) => res.json());
@@
- }, {
- headers: { cookie },
- }).then((res) => res.json());
+ }, {
+ headers,
+ }).then((res) => res.json());Also applies to: 55-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/loaders/orders/orderplaced.ts` around lines 42 - 43, The
resolveOrderCookie function currently returns an empty string when no eligible
cookies are found, causing an empty "cookie" header to be forwarded; change
resolveOrderCookie to return undefined instead of "" when no cookies are
present, and update the caller that forwards the header (the code that sets the
"cookie" request/header using the return of resolveOrderCookie) to only add the
"cookie" header when the value is !== undefined (i.e., guard the header
assignment so it is omitted entirely when resolveOrderCookie returns undefined).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="vtex/loaders/orders/orderplaced.ts">
<violation number="1" location="vtex/loaders/orders/orderplaced.ts:32">
P1: This condition passes when only one of the two required checkout cookies (`CheckoutDataAccess`, `Vtex_CHKO_Auth`) is present, skipping the auth-cookie fallback. Since VTEX requires both checkout cookies to grant access, a partial set will still cause the request to fail. Check that both cookies are present before choosing checkout-cookie mode.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ), | ||
| ); | ||
|
|
||
| if (Object.keys(checkoutCookies).length > 0) { |
There was a problem hiding this comment.
P1: This condition passes when only one of the two required checkout cookies (CheckoutDataAccess, Vtex_CHKO_Auth) is present, skipping the auth-cookie fallback. Since VTEX requires both checkout cookies to grant access, a partial set will still cause the request to fail. Check that both cookies are present before choosing checkout-cookie mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vtex/loaders/orders/orderplaced.ts, line 32:
<comment>This condition passes when only one of the two required checkout cookies (`CheckoutDataAccess`, `Vtex_CHKO_Auth`) is present, skipping the auth-cookie fallback. Since VTEX requires both checkout cookies to grant access, a partial set will still cause the request to fail. Check that both cookies are present before choosing checkout-cookie mode.</comment>
<file context>
@@ -11,6 +11,37 @@ interface Props {
+ ),
+ );
+
+ if (Object.keys(checkoutCookies).length > 0) {
+ return stringify(checkoutCookies);
+ }
</file context>
| if (Object.keys(checkoutCookies).length > 0) { | |
| const hasBothCheckoutCookies = | |
| checkoutCookies[CHECKOUT_DATA_ACCESS_COOKIE] != null && | |
| checkoutCookies[VTEX_CHKO_AUTH] != null; | |
| if (hasBothCheckoutCookies) { |
1 participant
Problem
After an order is placed, VTEX sets two short-lived cookies (
CheckoutDataAccessandVtex_CHKO_Auth) specifically to grant access to the order-placed page. However, the loader was sending those together withVtexIdclientAutCookie_*, which can still exist in the browser even after it has expired. Having an expired auth cookie in the request was causing it to fail.Fix
The loader now prioritizes checkout cookies when they're present, using only them for the request. The auth cookie is only used as a fallback when the checkout cookies are absent.
Summary by cubic
Prioritize checkout cookies in the order-placed loader to prevent failures caused by stale auth cookies. When available, the loader now sends only CheckoutDataAccess and Vtex_CHKO_Auth, falling back to VtexIdclientAutCookie_* only if needed.
Written for commit 9d11d70. Summary will update on new commits.
Summary by CodeRabbit