feat: support cross-subdomain redirect impression tracking#238
Conversation
|
📝 Documentation updates detected! New suggestion: Document cross-subdomain redirect impression tracking for URL redirect testing |
6e5ab5c to
0bc57bf
Compare
6d36199 to
fc10097
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unawaited async
fireStoredRedirectImpressionsinapplyVariants- Added
awaitbeforefireStoredRedirectImpressions()call at line 511 inapplyVariantsto ensure cookie-based redirect impressions are loaded before the for-loop processes variants and potentially triggers a redirect.
- Added
- ✅ Fixed: Async
applyVariantsnot awaited in subscription handlers- Made groupSubscribe callbacks async and added
awaitbeforeapplyVariants()andpreviewVariants()calls in both subscription-refactor.ts and subscriptions.ts to prevent race conditions and unhandled promise rejections.
- Made groupSubscribe callbacks async and added
Or push these changes by commenting:
@cursor push d7c8547784
Preview (d7c8547784)
diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts
--- a/packages/experiment-tag/src/experiment.ts
+++ b/packages/experiment-tag/src/experiment.ts
@@ -508,7 +508,7 @@
this.urlExposureCache[currentUrl] = {};
}
- this.fireStoredRedirectImpressions();
+ await this.fireStoredRedirectImpressions();
for (const key in variants) {
// preview actions are handled by previewVariants
if ((flagKeys && !flagKeys.includes(key)) || this.previewFlags[key]) {
diff --git a/packages/experiment-tag/src/subscriptions/subscription-refactor.ts b/packages/experiment-tag/src/subscriptions/subscription-refactor.ts
--- a/packages/experiment-tag/src/subscriptions/subscription-refactor.ts
+++ b/packages/experiment-tag/src/subscriptions/subscription-refactor.ts
@@ -186,7 +186,7 @@
// Set up group callbacks (one per trigger type)
for (const triggerType of Object.keys(triggerTypeExperimentMap)) {
- this.messageBus.groupSubscribe(triggerType as MessageType, (payload) => {
+ this.messageBus.groupSubscribe(triggerType as MessageType, async (payload) => {
const isUrlChange = triggerType === 'url_change';
// Handle URL change: reset state and revert injections
@@ -217,7 +217,7 @@
: Array.from(triggerTypeExperimentMap[triggerType] || []);
// Apply non-preview variants
- this.webExperimentClient.applyVariants({
+ await this.webExperimentClient.applyVariants({
flagKeys: relevantFlags?.filter(
(flag) => !this.webExperimentClient.previewFlags[flag],
),
@@ -233,7 +233,7 @@
)
: this.webExperimentClient.previewFlags;
- this.webExperimentClient.previewVariants({
+ await this.webExperimentClient.previewVariants({
keyToVariant: previewFlags,
});
}
diff --git a/packages/experiment-tag/src/subscriptions/subscriptions.ts b/packages/experiment-tag/src/subscriptions/subscriptions.ts
--- a/packages/experiment-tag/src/subscriptions/subscriptions.ts
+++ b/packages/experiment-tag/src/subscriptions/subscriptions.ts
@@ -374,7 +374,7 @@
// Set up groupCallbacks (one per trigger type)
for (const triggerType of Object.keys(triggerTypeExperimentMap)) {
- this.messageBus.groupSubscribe(triggerType as MessageType, (payload) => {
+ this.messageBus.groupSubscribe(triggerType as MessageType, async (payload) => {
const isUrlChange = triggerType === 'url_change';
const isAnalyticsEvent = triggerType === 'analytics_event';
@@ -443,7 +443,7 @@
}
// Apply non-preview variants
- this.webExperimentClient.applyVariants({
+ await this.webExperimentClient.applyVariants({
flagKeys: relevantFlags?.filter(
(flag) => !this.webExperimentClient.previewFlags[flag],
),
@@ -459,7 +459,7 @@
)
: this.webExperimentClient.previewFlags;
- this.webExperimentClient.previewVariants({
+ await this.webExperimentClient.previewVariants({
keyToVariant: previewFlags,
});
}You can send follow-ups to the cloud agent here.
… sites Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed:
.catch()without handler doesn't suppress rejections- Added empty callback
() => {}to all three.catch()calls to properly suppress promise rejections fromfireStoredRedirectImpressions().
- Added empty callback
Or push these changes by commenting:
@cursor push d46ee3ce98
Preview (d46ee3ce98)
diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts
--- a/packages/experiment-tag/src/experiment.ts
+++ b/packages/experiment-tag/src/experiment.ts
@@ -401,10 +401,10 @@
// fire stored redirect impressions upon startup (must run before applyVariants
// so the current URL is checked before any redirect changes location.href)
- this.fireStoredRedirectImpressions().catch();
+ this.fireStoredRedirectImpressions().catch(() => {});
// Subscribe directly to url_change events to fire redirect impressions
this.messageBus.subscribe('url_change', () => {
- this.fireStoredRedirectImpressions().catch();
+ this.fireStoredRedirectImpressions().catch(() => {});
});
// apply local variants
@@ -508,7 +508,7 @@
this.urlExposureCache[currentUrl] = {};
}
- await this.fireStoredRedirectImpressions().catch();
+ await this.fireStoredRedirectImpressions().catch(() => {});
for (const key in variants) {
// preview actions are handled by previewVariants
if ((flagKeys && !flagKeys.includes(key)) || this.previewFlags[key]) {You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed:
getTopLevelDomainuses barelocationnotglobalScope- Updated getTopLevelDomain and setMarketingCookie to accept an optional globalScope parameter (defaulting to getGlobalScope()) and use globalScope.location instead of bare location, ensuring consistency with the rest of the SDK.
Or push these changes by commenting:
@cursor push 2ed4a721de
Preview (2ed4a721de)
diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts
--- a/packages/experiment-tag/src/experiment.ts
+++ b/packages/experiment-tag/src/experiment.ts
@@ -935,7 +935,7 @@
// set previous url - relevant for SPA if redirect happens before push/replaceState is complete
this.previousUrl = this.globalScope.location.href;
- await setMarketingCookie(this.apiKey);
+ await setMarketingCookie(this.apiKey, this.globalScope);
// perform redirection
if (this.customRedirectHandler) {
this.customRedirectHandler(targetUrl);
@@ -1170,7 +1170,7 @@
// Also write to cookie when opted in, enabling cross-subdomain tracking
if (this.config.redirectConfig?.encodeRedirectInCookie) {
- const domain = await getTopLevelDomain();
+ const domain = await getTopLevelDomain(this.globalScope);
const storage = new CookieStorage<
Record<string, StoredRedirectImpression>
>({
@@ -1228,7 +1228,7 @@
| CookieStorage<Record<string, StoredRedirectImpression>>
| undefined;
if (this.config.redirectConfig?.encodeRedirectInCookie) {
- const domain = await getTopLevelDomain();
+ const domain = await getTopLevelDomain(this.globalScope);
cookieStorage = new CookieStorage<
Record<string, StoredRedirectImpression>
>({
diff --git a/packages/experiment-tag/src/util/cookie.ts b/packages/experiment-tag/src/util/cookie.ts
--- a/packages/experiment-tag/src/util/cookie.ts
+++ b/packages/experiment-tag/src/util/cookie.ts
@@ -2,6 +2,7 @@
import { CookieStorage } from '@amplitude/analytics-core';
import { MKTG } from '@amplitude/analytics-core';
import type { Campaign } from '@amplitude/analytics-core';
+import { getGlobalScope } from '@amplitude/experiment-core';
const KNOWN_2LDS = [
'ac.in',
@@ -122,9 +123,12 @@
let cachedDomain: string | undefined;
-export async function getTopLevelDomain(): Promise<string> {
+export async function getTopLevelDomain(
+ globalScope: typeof globalThis | undefined = getGlobalScope(),
+): Promise<string> {
if (cachedDomain !== undefined) return cachedDomain;
- if (typeof location === 'undefined' || !location.hostname) {
+ const location = globalScope?.location;
+ if (!location?.hostname) {
return (cachedDomain = '');
}
const host = location.hostname;
@@ -144,8 +148,11 @@
return (cachedDomain = '');
}
-export async function setMarketingCookie(apiKey: string) {
- const domain = await getTopLevelDomain();
+export async function setMarketingCookie(
+ apiKey: string,
+ globalScope: typeof globalThis | undefined = getGlobalScope(),
+) {
+ const domain = await getTopLevelDomain(globalScope);
const storage = new CookieStorage<Campaign>({
sameSite: 'Lax',
...(domain && { domain }),You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: URL param stripped even when parsing fails causing data loss
- Moved history.replaceState inside the try block so the URL parameter is only removed after successful JSON.parse(atob(encoded)), preserving the data for retry if parsing fails.
Or push these changes by commenting:
@cursor push 71363ed3bf
Preview (71363ed3bf)
diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts
--- a/packages/experiment-tag/src/experiment.ts
+++ b/packages/experiment-tag/src/experiment.ts
@@ -1206,14 +1206,14 @@
if (encoded) {
try {
urlImpressions = JSON.parse(atob(encoded));
+ this.globalScope.history.replaceState(
+ {},
+ '',
+ removeQueryParams(this.globalScope.location.href, [
+ REDIRECT_IMPRESSION_PARAM,
+ ]),
+ );
} catch {} // eslint-disable-line no-empty
- this.globalScope.history.replaceState(
- {},
- '',
- removeQueryParams(this.globalScope.location.href, [
- REDIRECT_IMPRESSION_PARAM,
- ]),
- );
}
}You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: btoa throws on non-ASCII variant metadata
- Added Unicode-safe base64 encoding/decoding using TextEncoder/TextDecoder to handle non-ASCII characters in variant metadata, replacing the direct btoa/atob calls at all three locations in the redirect impression handling flow.
Or push these changes by commenting:
@cursor push 8c7eaf3cb1
Preview (8c7eaf3cb1)
diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts
--- a/packages/experiment-tag/src/experiment.ts
+++ b/packages/experiment-tag/src/experiment.ts
@@ -872,6 +872,20 @@
}
}
+ private stringToBase64(str: string): string {
+ const bytes = new TextEncoder().encode(str);
+ const binString = Array.from(bytes, (byte) =>
+ String.fromCodePoint(byte),
+ ).join('');
+ return btoa(binString);
+ }
+
+ private base64ToString(base64: string): string {
+ const binString = atob(base64);
+ const bytes = Uint8Array.from(binString, (m) => m.codePointAt(0) ?? 0);
+ return new TextDecoder().decode(bytes);
+ }
+
private async handleRedirect(action, flagKey: string, variant: Variant) {
if (!this.isActionActiveOnPage(flagKey, action?.data?.metadata?.scope)) {
return;
@@ -913,7 +927,7 @@
);
if (existingEncoded) {
try {
- urlPayload = JSON.parse(atob(existingEncoded));
+ urlPayload = JSON.parse(this.base64ToString(existingEncoded));
} catch (error) {
console.error('Failed to decode existing AMP_REDIRECT param:', error);
}
@@ -928,7 +942,7 @@
};
targetUrlObj.searchParams.set(
REDIRECT_IMPRESSION_PARAM,
- btoa(JSON.stringify(urlPayload)),
+ this.stringToBase64(JSON.stringify(urlPayload)),
);
targetUrl = targetUrlObj.toString();
}
@@ -1205,7 +1219,7 @@
const encoded = urlParams[REDIRECT_IMPRESSION_PARAM];
if (encoded) {
try {
- urlImpressions = JSON.parse(atob(encoded));
+ urlImpressions = JSON.parse(this.base64ToString(encoded));
} catch {} // eslint-disable-line no-empty
this.globalScope.history.replaceState(
{},You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cookie redirect enabled by default
- Changed encodeRedirectInCookie default from true to false in the Defaults object to make cookie storage opt-in as per the PR's stated behavior.
Or push these changes by commenting:
@cursor push 6e835272d3
Preview (6e835272d3)
diff --git a/packages/experiment-tag/src/types.ts b/packages/experiment-tag/src/types.ts
--- a/packages/experiment-tag/src/types.ts
+++ b/packages/experiment-tag/src/types.ts
@@ -135,7 +135,7 @@
useDefaultNavigationHandler: true,
redirectConfig: {
encodeRedirectInUrl: false,
- encodeRedirectInCookie: true,
+ encodeRedirectInCookie: false,
},
};You can send follow-ups to the cloud agent here.
…prevent spurious url_change
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Page subscribers fire every trigger
- Added the missing
if (pagesChanged)guard around page change subscriber notification to match the correct pattern in subscription-refactor.ts.
- Added the missing
- ✅ Fixed: Public suffix cookie domain
- Normalized hostname to lowercase before checking against KNOWN_2LDS to ensure case-insensitive matching prevents public suffixes like .co.uk from being probed first.
Or push these changes by commenting:
@cursor push a690112053
Preview (a690112053)
diff --git a/packages/experiment-tag/src/subscriptions/subscriptions.ts b/packages/experiment-tag/src/subscriptions/subscriptions.ts
--- a/packages/experiment-tag/src/subscriptions/subscriptions.ts
+++ b/packages/experiment-tag/src/subscriptions/subscriptions.ts
@@ -468,9 +468,11 @@
}
// Notify subscribers if pages actually changed
- this.lastNotifiedActivePages = clonePageObjects(activePages);
- for (const subscriber of this.pageChangeSubscribers) {
- subscriber({ activePages });
+ if (pagesChanged) {
+ this.lastNotifiedActivePages = clonePageObjects(activePages);
+ for (const subscriber of this.pageChangeSubscribers) {
+ subscriber({ activePages });
+ }
}
// Debug subscribers fire on any URL change or page change,
diff --git a/packages/experiment-tag/src/util/cookie.ts b/packages/experiment-tag/src/util/cookie.ts
--- a/packages/experiment-tag/src/util/cookie.ts
+++ b/packages/experiment-tag/src/util/cookie.ts
@@ -130,7 +130,8 @@
const parts = hostname.split('.');
if (parts.length === 1) return (cachedDomain = '');
- const skipLevel = KNOWN_2LDS.some((tld) => hostname.endsWith(`.${tld}`))
+ const lowerHostname = hostname.toLowerCase();
+ const skipLevel = KNOWN_2LDS.some((tld) => lowerHostname.endsWith(`.${tld}`))
? 2
: 1;
const levels: string[] = [];You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 16a9c66. Configure here.
…main redirects Skip cookie and AMP_REDIRECT URL param encoding when the redirect target hostname matches the current hostname — sessionStorage is sufficient for same-subdomain redirects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>


Summary
Adds opt-in support for cross-subdomain and cross-domain redirect impression tracking without changing default behavior for existing customers. Two new flags on
redirectConfigcontrol the fallback strategies:By default, redirect impressions continue to use
sessionStorageonly. When either flag is set, impressions are additionally persisted via the configured fallback(s) so they survive the redirect to a different subdomain or domain.Fallback strategies:
encodeRedirectInCookie— writes impression data to a short-lived cookie scoped to the root domain (resolved viagetTopLevelDomain()). Enables tracking across subdomains that share a root domain (e.g.app.example.com→www.example.com). Cookie is only written when the redirect target hostname differs from the current hostname — same-subdomain redirects use sessionStorage only.encodeRedirectInUrl— base64-encodes impression data into anAMP_REDIRECTquery parameter appended to the redirect URL. Enables tracking across domains and in cookie-blocked environments. The param is cleaned from the URL viahistory.replaceStateafter impressions are fired. URL param is only appended when the redirect target hostname differs from the current hostname.On the destination page,
fireStoredRedirectImpressionsmerges all three sources with priority: URL > sessionStorage > cookie.Supporting changes:
getTopLevelDomain()(util/cookie.ts): New async utility that probes for the broadest writable cookie domain. Includes aKNOWN_2LDSlist to skip known public two-level suffixes (e.g..co.uk). Result is module-level cached after the first call. Used to scope both the marketing cookie and redirect impression cookies.isCrossSubdomainis computed inhandleRedirectby comparingnew URL(targetUrl).hostnamewithlocation.hostname. Cookie writes and URL param encoding are skipped entirely for same-hostname redirects — only sessionStorage is used.StoredRedirectImpressiontype: Replaces storing the fullVariantobject with a minimal shape ({ redirectUrl, variantKey, expKey?, metadata? }) — only the fields needed to reconstruct the exposure event.setMarketingCookie: Now resolves its own domain internally viagetTopLevelDomain()— no longer accepts adomainparameter.fireStoredRedirectImpressionsare nowasync, ensuring all impression storage writes are fully awaited beforelocation.replacefires.Checklist
Note
Medium Risk
Touches core redirect, exposure deduplication, and startup ordering (anti-flicker, async applyVariants); mis-merged impressions or URL cleanup could cause missed or duplicate exposures, though defaults preserve existing sessionStorage-only behavior.
Overview
Adds opt-in
redirectConfigon web experiments so redirect exposure/impression data can survive navigation beyondsessionStorage, without changing behavior unless customers enable the flags.When
encodeRedirectInCookieis on and the redirect crosses hostnames, impressions are written to a short-lived root-domain cookie (via newgetTopLevelDomain()). WhenencodeRedirectInUrlis on, data is merged into a base64AMP_REDIRECTquery param on the destination URL and stripped withreplaceStateafter firing (withmarkUrlAsPublishedto avoid duplicate exposures). On landing,fireStoredRedirectImpressionsmerges sources with priority URL > session > cookie, uses a leanStoredRedirectImpressionpayload instead of full variants, and runs earlier onstartplusurl_changesubscriptions.Redirect handling is now async (
applyVariants,previewVariants,handleRedirect): storage and marketing cookie writes are awaited beforelocation.replace.setMarketingCookiescopes cookies using the resolved top-level domain. Subscription group callbacksawaitvariant application. Tests addCookieStoragemocks and cross-subdomain/cross-domain redirect cases.Reviewed by Cursor Bugbot for commit 8cb4da1. Bugbot is set up for automated code reviews on this repo. Configure here.