Merge pull request #53 from decocms/fix/headers-aware-cookie-merge

fix(vtex): merge headers Headers-aware so init.headers Cookie survives

Author: vibe-dex

package-lock.json

@@ -7,7 +7,14 @@
 
 import { RequestContext } from "@decocms/start/sdk/requestContext";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-import { configureVtex, setVtexFetch, vtexFetchResponse } from "../client";
+import {
+	configureVtex,
+	intelligentSearch,
+	setVtexFetch,
+	vtexCachedFetch,
+	vtexFetchResponse,
+} from "../client";
+import { clearFetchCache } from "../utils/fetchCache";
 
 function mockResponse(body: unknown = {}, status = 200): Response {
 	return {
@@ -43,6 +50,25 @@ function mockResponse(body: unknown = {}, status = 200): Response {
  * `vi.spyOn`. The spy is restored after `fn` resolves to keep tests
  * isolated. Nothing here depends on undici or ALS internals.
  */
+/**
+ * Read a header from an init in a shape-agnostic way. After the
+ * `mergeHeaders` refactor, `init.headers` is always a `Headers`
+ * instance — but the helper handles legacy shapes too so the tests
+ * stay robust if someone changes the merge implementation again.
+ */
+function headerValue(init: RequestInit | undefined, name: string): string | undefined {
+	const headers = init?.headers;
+	if (!headers) return undefined;
+	if (headers instanceof Headers) return headers.get(name) ?? undefined;
+	if (Array.isArray(headers)) {
+		const found = headers.find(([k]) => k.toLowerCase() === name.toLowerCase());
+		return found?.[1];
+	}
+	const rec = headers as Record<string, string>;
+	const key = Object.keys(rec).find((k) => k.toLowerCase() === name.toLowerCase());
+	return key ? rec[key] : undefined;
+}
+
 function withRequest<T>(cookieHeader: string | null, fn: () => Promise<T>): Promise<T> {
 	const headers = new Headers();
 	if (cookieHeader) headers.set("cookie", cookieHeader);
@@ -79,8 +105,7 @@ describe("vtexFetchResponse — vtex_segment cookie forwarding", () => {
 		await withRequest("vtex_segment=abc123; other=foo", async () => {
 			await vtexFetchResponse("/api/catalog_system/pub/products/x");
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers.cookie).toBe("vtex_segment=abc123");
+		expect(headerValue(lastInit, "cookie")).toBe("vtex_segment=abc123");
 	});
 
 	it("does not overwrite a caller-supplied cookie header", async () => {
@@ -89,8 +114,7 @@ describe("vtexFetchResponse — vtex_segment cookie forwarding", () => {
 				headers: { cookie: "custom=zzz" },
 			});
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers.cookie).toBe("custom=zzz");
+		expect(headerValue(lastInit, "cookie")).toBe("custom=zzz");
 	});
 
 	it("does not overwrite a caller-supplied Cookie header (case-insensitive)", async () => {
@@ -99,44 +123,133 @@ describe("vtexFetchResponse — vtex_segment cookie forwarding", () => {
 				headers: { Cookie: "custom=zzz" },
 			});
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		// The merged headers may contain both keys; the caller's value should win.
-		const lowered = Object.fromEntries(
-			Object.entries(headers).map(([k, v]) => [k.toLowerCase(), v]),
-		);
-		expect(lowered.cookie).toBe("custom=zzz");
+		expect(headerValue(lastInit, "cookie")).toBe("custom=zzz");
 	});
 
 	it("is a no-op when there is no incoming cookie header", async () => {
 		await withRequest(null, async () => {
 			await vtexFetchResponse("/api/x");
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers.cookie).toBeUndefined();
+		expect(headerValue(lastInit, "cookie")).toBeUndefined();
 	});
 
 	it("is a no-op when there is a cookie header but no vtex_segment", async () => {
 		await withRequest("other=foo; another=bar", async () => {
 			await vtexFetchResponse("/api/x");
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers.cookie).toBeUndefined();
+		expect(headerValue(lastInit, "cookie")).toBeUndefined();
 	});
 
 	it("does not crash when called outside a RequestContext", async () => {
 		await vtexFetchResponse("/api/x");
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers.cookie).toBeUndefined();
+		expect(headerValue(lastInit, "cookie")).toBeUndefined();
 	});
 
 	it("preserves auth headers alongside the forwarded cookie", async () => {
 		configureVtex({ account: "testaccount", appKey: "k", appToken: "t" });
 		await withRequest("vtex_segment=abc123", async () => {
 			await vtexFetchResponse("/api/x");
 		});
-		const headers = lastInit?.headers as Record<string, string>;
-		expect(headers["X-VTEX-API-AppKey"]).toBe("k");
-		expect(headers["X-VTEX-API-AppToken"]).toBe("t");
-		expect(headers.cookie).toBe("vtex_segment=abc123");
+		expect(headerValue(lastInit, "X-VTEX-API-AppKey")).toBe("k");
+		expect(headerValue(lastInit, "X-VTEX-API-AppToken")).toBe("t");
+		expect(headerValue(lastInit, "cookie")).toBe("vtex_segment=abc123");
+	});
+
+	// Regression: when init.headers is a Headers object (as
+	// `createVtexCheckoutProxy` passes through `getVtexFetch()`), the
+	// existing cookie must survive verbatim. The naive
+	// `{ ...authHeaders, ...init?.headers }` spread collapses a Headers
+	// instance to `{}` (Headers has no own enumerable entries), which
+	// silently wipes the browser's full Cookie header — including the
+	// orderForm cookie any checkout flow depends on.
+	it("preserves an existing Cookie header when init.headers is a Headers instance", async () => {
+		await withRequest("vtex_segment=abc123", async () => {
+			const proxyInit: RequestInit = {
+				headers: new Headers({
+					cookie: "checkout.vtex.com=__ofid=xyz; vtex_segment=originalseg; foo=bar",
+				}),
+			};
+			await vtexFetchResponse("/api/checkout/pub/orderForm", proxyInit);
+		});
+		expect(headerValue(lastInit, "cookie")).toContain("checkout.vtex.com=__ofid=xyz");
+	});
+});
+
+// Module-level counter for cache-busting test URLs. `Date.now()` collides
+// when two tests run within the same millisecond and the SWR cache in
+// fetchWithCache short-circuits the second one — `_fetch` never runs and
+// `lastInit` stays `undefined`. Per-test ids are deterministic and
+// collision-free.
+let testUrlCounter = 0;
+const uniqPath = (prefix: string) => `${prefix}/${++testUrlCounter}`;
+
+describe("vtexCachedFetch — vtex_segment cookie forwarding", () => {
+	let lastInit: RequestInit | undefined;
+
+	beforeEach(() => {
+		clearFetchCache();
+		configureVtex({ account: "testaccount" });
+		lastInit = undefined;
+		setVtexFetch(((_url: string, init?: RequestInit) => {
+			lastInit = init;
+			return Promise.resolve(mockResponse({ ok: true }));
+		}) as typeof fetch);
+	});
+
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
+	it("forwards vtex_segment on cached GETs", async () => {
+		await withRequest("vtex_segment=abc123", async () => {
+			await vtexCachedFetch(uniqPath("/api/catalog_system/pub/products"));
+		});
+		expect(headerValue(lastInit, "cookie")).toBe("vtex_segment=abc123");
+	});
+
+	it("does not overwrite a caller-supplied cookie header", async () => {
+		await withRequest("vtex_segment=abc123", async () => {
+			await vtexCachedFetch(uniqPath("/api/x"), {
+				headers: { cookie: "custom=zzz" },
+			});
+		});
+		expect(headerValue(lastInit, "cookie")).toBe("custom=zzz");
+	});
+});
+
+describe("intelligentSearch — vtex_segment cookie forwarding", () => {
+	let lastInit: RequestInit | undefined;
+
+	beforeEach(() => {
+		// Reset the SWR cache: otherwise the second test in this block
+		// can serve the first test's cached body without invoking the
+		// stub _fetch, leaving lastInit undefined.
+		clearFetchCache();
+		configureVtex({ account: "testaccount" });
+		lastInit = undefined;
+		setVtexFetch(((_url: string, init?: RequestInit) => {
+			lastInit = init;
+			return Promise.resolve(mockResponse({ products: [] }));
+		}) as typeof fetch);
+	});
+
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
+	it("forwards vtex_segment when caller didn't pass cookieHeader", async () => {
+		await withRequest("vtex_segment=abc123; other=foo", async () => {
+			await intelligentSearch(uniqPath("/product_search"));
+		});
+		expect(headerValue(lastInit, "cookie")).toBe("vtex_segment=abc123");
+	});
+
+	it("respects an explicit cookieHeader override", async () => {
+		await withRequest("vtex_segment=abc123", async () => {
+			await intelligentSearch(uniqPath("/product_search"), undefined, {
+				cookieHeader: "custom=zzz",
+			});
+		});
+		expect(headerValue(lastInit, "cookie")).toBe("custom=zzz");
 	});
 });

vtex/__tests__/client-segment-cookie.test.ts

@@ -241,18 +241,45 @@ export async function vtexFetchResponse(
 
 	const response = await _fetch(url, {
 		...init,
-		headers: {
-			...authHeaders(),
-			...(segmentCookie ? { cookie: segmentCookie } : {}),
-			...init?.headers,
-		},
+		headers: mergeHeaders(authHeaders(), segmentCookie, init?.headers),
 	});
 	if (!response.ok) {
 		throw new Error(`VTEX API error: ${response.status} ${response.statusText} - ${url}`);
 	}
 	return response;
 }
 
+/**
+ * Combine framework headers + optional segment cookie + caller headers,
+ * preserving the precedence "caller wins" regardless of whether the
+ * caller passed `Headers`, `string[][]`, or `Record<string, string>`.
+ *
+ * Why a helper: the naive `{ ...authHeaders, ...init?.headers }` spread
+ * silently collapses a `Headers` instance to `{}` (Headers has no own
+ * enumerable entries), which means any cookies the caller put on a
+ * Headers object are lost on the wire. The `createVtexCheckoutProxy`
+ * factory passes init with Headers, which makes this the failure mode
+ * for every forwarder that relies on browser-supplied cookies reaching
+ * VTEX. Funneling all merges through the `Headers` constructor (which
+ * correctly absorbs every HeadersInit shape) keeps the bug from
+ * sneaking back in.
+ */
+function mergeHeaders(
+	auth: Record<string, string>,
+	segmentCookie: string | null,
+	callerHeaders: HeadersInit | undefined,
+): Headers {
+	const merged = new Headers(auth);
+	if (segmentCookie) merged.set("cookie", segmentCookie);
+	if (callerHeaders) {
+		const incoming = new Headers(callerHeaders);
+		incoming.forEach((value, key) => {
+			merged.set(key, value);
+		});
+	}
+	return merged;
+}
+
 export async function vtexFetch<T>(path: string, init?: InstrumentedFetchInit): Promise<T> {
 	const response = await vtexFetchResponse(path, init);
 	return response.json();
@@ -282,12 +309,22 @@ export async function vtexCachedFetch<T>(
 		? { ttl: cacheOpts.cacheTTL }
 		: undefined;
 
+	// Mirrors vtexFetchResponse: Legacy Catalog and several other GET
+	// endpoints gate regional seller availability on the `vtex_segment`
+	// cookie. Cached GETs (PDP / shelf product lookups) must see the same
+	// regionalization the rest of the stack does — otherwise sites have
+	// to wrap _fetch themselves to forward the cookie, which is easy to
+	// get subtly wrong (especially around HeadersInit shapes). Inline
+	// here keeps the surface small; if a third callsite appears we
+	// extract a shared helper.
+	const segmentCookie = !hasCookieHeader(init?.headers) ? getSegmentCookieHeader() : null;
+
 	return fetchWithCache<T>(
 		url,
 		() =>
 			_fetch(url, {
 				...init,
-				headers: { ...authHeaders(), ...init?.headers },
+				headers: mergeHeaders(authHeaders(), segmentCookie, init?.headers),
 			}),
 		opts,
 	);
@@ -383,6 +420,14 @@ export async function intelligentSearch<T>(
 	const headers: Record<string, string> = { ...authHeaders() };
 	if (opts?.cookieHeader) {
 		headers.cookie = opts.cookieHeader;
+	} else {
+		// IS already gets regionId on the query string above, but some
+		// internal IS flows (and downstream services it consults) still
+		// honor the `vtex_segment` cookie — forward it when the caller
+		// didn't pass an explicit one. See vtexCachedFetch for the same
+		// rationale.
+		const segmentCookie = getSegmentCookieHeader();
+		if (segmentCookie) headers.cookie = segmentCookie;
 	}
 
 	const fullUrl = url.toString();