diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index 2b630c8751..d64f1b2516 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -1,5 +1,6 @@ import { Alert, Badge, Box, Button, Center, Group, Select, Stack, Text } from '@mantine/core' -import React from 'react' +import React, { useEffect, useRef, useState } from 'react' +import { toErrorMessage } from '../../../../common/toErrorMessage' import { useSettings } from '../../../hooks/useSettings' import type { FlagAuthState } from './useFlagAuth' import { useInspectedPageOverrides } from './useInspectedPageOverrides' @@ -41,17 +42,64 @@ export function ConnectScreen({ auth }: { auth: FlagAuthState }) { } /** - * Surfaces overrides already stored on the inspected page while signed out — otherwise this screen is - * all that renders, so an override left from an earlier session keeps affecting the page with nothing - * to explain it. Informational only; everything that mutates overrides lives on the connected tab. + * Surfaces overrides already stored on the inspected page while signed out, with a Clear all so they + * can be wiped without signing in. * * Mounted only while disconnected, so its navigation listeners never run alongside the connected * tab's own instance of this hook. */ function DisconnectedOverridesNotice() { - const { status, overrides } = useInspectedPageOverrides() + const { status, overrides, clearAll, reloadPage } = useInspectedPageOverrides() + const [phase, setPhase] = useState<'idle' | 'confirming' | 'clearing' | 'cleared'>('idle') + const [error, setError] = useState(null) + // Bumped on navigation so a clear that settles afterwards can't report on the page it left. + const generation = useRef(0) const count = Object.keys(overrides).length + // A navigation brings a fresh set of overrides: a pending confirmation would clear those instead, + // and a previous "cleared" no longer describes the page. + useEffect(() => { + if (status === 'loading') { + generation.current += 1 + setPhase('idle') + setError(null) + } + }, [status]) + + const handleClearAll = () => { + const started = generation.current + setPhase('clearing') + setError(null) + void clearAll() + .then(() => { + if (started === generation.current) { + setPhase('cleared') + } + }) + .catch((err: unknown) => { + if (started === generation.current) { + setError(toErrorMessage(err)) + setPhase('idle') + } + }) + } + + // Gated on the page agreeing it has none: a same-document SPA navigation fires no webNavigation + // event, so without this the banner could outlive the state it describes. + if (phase === 'cleared' && status === 'ready' && count === 0) { + // The page keeps applying them until it reloads and rebuilds its provider. + return ( + + + Please reload the page to stop applying them. + + + + ) + } + if (status !== 'ready' || count === 0) { return null } @@ -63,11 +111,42 @@ function DisconnectedOverridesNotice() { color="orange" w="100%" data-dd-privacy="mask" - title={`${count} override${count === 1 ? '' : 's'} active on this page`} + title={`${count} override${count === 1 ? '' : 's'} stored for this page`} > - These are stored in the page and keep applying while you are signed out. Sign in to view and remove them. + These stay in the page and may still be applying while you are signed out. Sign in to review and remove them + individually. + + {phase === 'confirming' || phase === 'clearing' ? ( + <> + + Clear all overrides on this page, including any saved for other Datadog sites? + + + + + ) : ( + + )} + + {error && ( + + Could not clear the overrides: {error} + + )} ) } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx index 99ece77997..b32bb40449 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx @@ -16,7 +16,7 @@ import { IconArrowBackUp, IconCopy } from '@tabler/icons-react' import React, { useLayoutEffect, useRef, useState, type ReactNode } from 'react' import type { CatalogFlag } from './flagsRequests' import { useFlagsContext } from './flagsContext' -import { validateOverrideValue } from './flagTypes' +import { flagTypeLabel, validateOverrideValue } from './flagTypes' import { getOverride, type FlagOverride } from './inspectedPageFlags' export function FlagCatalogBody() { @@ -121,6 +121,8 @@ function FlagRow({ onRevert: (flagKey: string) => void }) { const overridden = override !== undefined + // The wrapper rejects a mismatched type, so this override won't apply. + const typeMismatch = overridden && override.type !== flag.type return ( @@ -141,6 +147,18 @@ function FlagRow({ {flag.description && } + {override && typeMismatch && ( + + Type mismatch: stored as {flagTypeLabel(override.type)}, but this flag is {flagTypeLabel(flag.type)}. It + won't apply until you clear it. + + )} + {/* Not an error: the override still works, the flag just left the catalog. */} + {flag.unresolved && ( + + No active flag with this key on this site — it may have been archived or deleted. + + )} {overridden && ( @@ -156,7 +174,7 @@ function FlagRow({ ) : ( flag.variants.map((variant) => { - const isActive = overridden && valuesEqual(override.value, variant.value) + const isActive = overridden && !typeMismatch && valuesEqual(override.value, variant.value) // The catalog keeps an unparseable variant as its raw string (see parseVariantValue), and // writing that through would break the override type contract. `allowNull` keeps a // legitimate JSON `null` variant applyable. diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts index 376d247e1e..158a8f5585 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.spec.ts @@ -7,6 +7,13 @@ describe('flagTypeLabel', () => { // An API value_type we don't model yet must not crash — fall back to the raw type. expect(flagTypeLabel('MYSTERY' as FlagType)).toBe('MYSTERY') }) + + it('always returns a string, so a malformed stored type cannot crash the row rendering it', () => { + expect(flagTypeLabel({} as unknown as FlagType)).toBe('[object Object]') + expect(flagTypeLabel(undefined as unknown as FlagType)).toBe('undefined') + // Finds Object.prototype.toString rather than a config, so `label` is what must be checked. + expect(flagTypeLabel('toString' as FlagType)).toBe('toString') + }) }) describe('validateOverrideValue', () => { diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts index 7611ea848c..a8e4fa3d34 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagTypes.ts @@ -96,8 +96,13 @@ export function validateOverrideValue( return null } -/** Display label for a flag type, falling back to the raw type for one we don't model yet. */ +/** + * Display label for a flag type, falling back to the raw type for one we don't model yet. Always a + * string: sanitizeOverrides keeps a shaped-but-malformed override so it stays removable, so `type` + * can be a non-string, and returning that would crash the row rendering it. `label` is checked + * rather than `config` because a type named `toString` or `constructor` finds an inherited member. + */ export function flagTypeLabel(type: FlagType): string { const config: FlagTypeConfig | undefined = FLAG_TYPE_CONFIG[type] - return config ? config.label : type + return typeof config?.label === 'string' ? config.label : String(type) } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx index 8797854ad0..c5539f2140 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsContext.tsx @@ -26,6 +26,10 @@ export interface FlagsContextValue { totalPages: number /** Whether a refresh is needed/in flight to (re)apply overrides, and the last mutation failure. */ pendingReload: boolean + /** The page is still applying another site's overrides until it reloads. */ + siteSwitchNeedsReload: boolean + /** Set when scoping to the connected site failed, so the page may be applying another site's. */ + scopeError: string | null writesInFlight: number mutationError: string | null applyOverride: (flagKey: string, override: FlagOverride) => void @@ -56,8 +60,18 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre const view = useFlagCatalogView(identity.userId) const catalog = useFlagCatalog(auth, view.request) const { setPage } = view - const { status, error, overrides, devtoolsEnabled, setOverride, clearOverride, clearAll, reloadPage } = - useInspectedPageOverrides() + const { + status, + error, + overrides, + devtoolsEnabled, + setOverride, + clearOverride, + clearAll, + reloadPage, + siteSwitchNeedsReload, + scopeError, + } = useInspectedPageOverrides(auth.site) const totalPages = Math.max(1, Math.ceil(catalog.total / view.pageSize)) @@ -72,7 +86,7 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre // Fetched by key so an overridden flag shows regardless of which catalog page it's on, with a // minimal row as fallback for a key that no longer resolves (so it can still be reverted). const overrideKeys = useMemo(() => Object.keys(overrides), [overrides]) - const overriddenCatalogFlags = useOverriddenFlags(auth, overrideKeys) + const { flags: overriddenCatalogFlags, missingKeys } = useOverriddenFlags(auth, overrideKeys) const overriddenFlags = useMemo( () => overrideKeys.map( @@ -84,9 +98,10 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre type: overrides[key].type, variants: [], tags: [], + unresolved: missingKeys.has(key), } ), - [overrideKeys, overriddenCatalogFlags, overrides] + [overrideKeys, overriddenCatalogFlags, missingKeys, overrides] ) // Dropped from the paginated list so they don't show twice. const bottomFlags = useMemo( @@ -159,6 +174,8 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre tagSuggestions, totalPages, pendingReload, + siteSwitchNeedsReload, + scopeError, writesInFlight, mutationError, applyOverride, @@ -179,6 +196,8 @@ export function FlagsProvider({ auth, children }: { auth: FlagAuthState; childre tagSuggestions, totalPages, pendingReload, + siteSwitchNeedsReload, + scopeError, writesInFlight, mutationError, applyOverride, diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts index 07c55070ab..d656a961f3 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.spec.ts @@ -247,22 +247,35 @@ describe('flagsRequests', () => { return Promise.resolve(new Response(JSON.stringify({ data }))) }) - const flags = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'missing', 'flag-b']) + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'missing', 'flag-b']) expect(spy).toHaveBeenCalledTimes(3) const firstUrl = new URL(spy.calls.argsFor(0)[0] as string) expect(firstUrl.searchParams.get('key')).toBe('flag-a') + // Active-only: an archived flag sharing the key would win the dedupe and describe the override + // against the wrong type and variants. expect(firstUrl.searchParams.get('is_archived')).toBe('false') expect(flags.map((flag) => flag.key)).toEqual(['flag-a', 'flag-b']) + expect(missingKeys).toEqual(['missing']) + }) + + it('does not call a key missing when the response body has no data array', async () => { + // A 2xx that isn't the expected envelope (proxy interstitial, schema change) is tolerated so + // the section still renders, but it is not evidence the flag is gone. + spyOn(globalThis, 'fetch').and.returnValue(Promise.resolve(new Response(JSON.stringify({ errors: ['x'] })))) + + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a']) + expect(flags).toEqual([]) + expect(missingKeys).toEqual([]) }) it('makes no request and returns nothing for an empty key list', async () => { const spy = spyOn(globalThis, 'fetch') - expect(await fetchFlagsByKeys('tok', 'datad0g.com', [])).toEqual([]) + expect(await fetchFlagsByKeys('tok', 'datad0g.com', [])).toEqual({ flags: [], missingKeys: [] }) expect(spy).not.toHaveBeenCalled() }) - it('drops a key whose request fails but keeps the ones that resolve', async () => { + it('drops a key whose request fails, keeps the ones that resolve, and does not call it missing', async () => { spyOn(globalThis, 'fetch').and.callFake((input) => { const key = new URL(input as string).searchParams.get('key') if (key === 'boom') { @@ -273,8 +286,9 @@ describe('flagsRequests', () => { ) }) - const flags = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'boom', 'flag-b']) + const { flags, missingKeys } = await fetchFlagsByKeys('tok', 'datad0g.com', ['flag-a', 'boom', 'flag-b']) expect(flags.map((flag) => flag.key)).toEqual(['flag-a', 'flag-b']) + expect(missingKeys).toEqual([]) }) }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts index 729dd92a54..c27933537b 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsRequests.ts @@ -12,6 +12,8 @@ export interface CatalogFlag { tags: string[] /** Undefined for flags created by a service account or integration, which carry no user UUID. */ createdBy?: string + /** Synthesized locally, never from the API: no active flag on the connected site holds this key. */ + unresolved?: boolean } /** @@ -113,42 +115,65 @@ export function fetchFlagCatalog(token: string, site: string, request: FlagCatal return fetchFlagPage(url, token, 'Failed to fetch flag catalog') } +export interface FlagsByKeysResult { + flags: CatalogFlag[] + /** Keys whose lookup completed and matched nothing; a failed lookup is not in here. */ + missingKeys: string[] +} + /** * Fetches a specific set of flags by exact key, one request per key (the endpoint's `key` filter is * exact and single-valued — there's no batched lookup). Used for the "Local overrides" section, * which must show overridden flags even when they're not on the current catalog page. * * Settles per key rather than Promise.all: one key failing transiently must not discard the flags - * that did resolve, or the whole section collapses to bare fallback rows. Keys with no match - * (deleted, or an override for a non-existent flag) are simply absent, and the caller falls back to - * a minimal row. Error labels omit the key — it's customer data, kept out of logs. + * that did resolve, or the whole section collapses to bare fallback rows. Only a well-formed + * response that matched nothing counts as `missingKeys` — a failed request, or a 2xx whose body + * isn't the expected envelope, proves nothing. Error labels omit the key — it's customer data. + * + * Active-only for the same reason as the catalog: an archived and an active flag can share a key, + * and mapResources keeps whichever the server listed first, so including archived ones would risk + * describing the override against the wrong flag's type and variants. An override on a flag archived + * here therefore reads as absent, which the row reports as archived or deleted. */ -export async function fetchFlagsByKeys(token: string, site: string, keys: string[]): Promise { +export async function fetchFlagsByKeys(token: string, site: string, keys: string[]): Promise { const host = getFlagsApiHost(site) const results = await Promise.allSettled( - keys.map(async (key) => { + keys.map((key) => { const url = new URL(`https://${host}/api/ui/ffe/feature-flags`) url.searchParams.set('key', key) url.searchParams.set('is_archived', 'false') - const { flags } = await fetchFlagPage(url, token, 'Failed to fetch flag') - return flags + return fetchFlagPage(url, token, 'Failed to fetch flag') }) ) - return results.flatMap((result) => (result.status === 'fulfilled' ? result.value : [])) + const flags = results.flatMap((result) => (result.status === 'fulfilled' ? result.value.flags : [])) + const foundKeys = new Set(flags.map((flag) => flag.key)) + const missingKeys = keys.filter( + (key, index) => results[index].status === 'fulfilled' && results[index].value.wellFormed && !foundKeys.has(key) + ) + return { flags, missingKeys } } /** * Shared request/response handling for both fetch functions: run the request, tolerate a response * that omits or mistypes `data`, and map its resources. `total` falls back to the resource count * when the server omits `meta.page.total` (a partial response, or the by-key lookup which sends no - * pagination fields). + * pagination fields). `wellFormed` reports whether `data` was actually there, so a caller reading + * meaning into an empty result can tell a real no-match from a body we merely tolerated. */ -async function fetchFlagPage(url: URL, token: string, errorLabel: string): Promise { +async function fetchFlagPage( + url: URL, + token: string, + errorLabel: string +): Promise { const body = await fetchFfeJson(url.toString(), token, errorLabel) - const resources = Array.isArray(body?.data) ? body.data : [] + const data = body?.data + const wellFormed = Array.isArray(data) + const resources = wellFormed ? data : [] return { flags: mapResources(resources), total: body?.meta?.page?.total ?? resources.length, + wellFormed, } } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx index f402ac535b..9c1148ceab 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagsTab.tsx @@ -1,4 +1,4 @@ -import { Alert, Anchor, Box, Button, Code, Group, Pagination, Space, Title } from '@mantine/core' +import { Alert, Anchor, Box, Button, Code, Group, Pagination, Space, Text, Title } from '@mantine/core' import React, { useState } from 'react' import { TabBase } from '../../tabBase' import { ConnectScreen, ConnectionHeader } from './connectScreen' @@ -6,6 +6,7 @@ import { FlagCatalogBody, OverridesSection } from './flagCatalogList' import { FlagFilterBar } from './flagFilterBar' import { FlagsProvider, useFlagsContext } from './flagsContext' import { ManualOverrideForm } from './manualOverrideForm' +import { siteShortLabel } from './oauth' import type { FlagAuthState } from './useFlagAuth' import { useFlagAuth } from './useFlagAuth' @@ -40,6 +41,8 @@ function ConnectedFlagsTab({ auth }: { auth: FlagAuthState }) { totalPages, view, pendingReload, + siteSwitchNeedsReload, + scopeError, writesInFlight, mutationError, removeAll, @@ -64,6 +67,35 @@ function ConnectedFlagsTab({ auth }: { auth: FlagAuthState }) { } > + {siteSwitchNeedsReload && ( + <> + + + The page is currently loaded with a different set of overrides. + + + + + + )} + + {scopeError && ( + <> + + {scopeError} + + + + )} + {mutationError && ( <> diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts index 3a2d26bb56..8eaca0f799 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.spec.ts @@ -5,9 +5,27 @@ import { clearAllOverrides, deleteOverride, readFlagState, + siteOverridesKey, + syncSiteOverrides, writeOverride, } from './inspectedPageFlags' +const STAGING = 'datad0g.com' +const US1 = 'datadoghq.com' + +function clearSiteStores() { + for (let i = localStorage.length - 1; i >= 0; i--) { + const key = localStorage.key(i) + if (key?.startsWith(`${OVERRIDES_KEY}.`)) { + localStorage.removeItem(key) + } + } +} + +function stored(key: string): unknown { + return JSON.parse(localStorage.getItem(key) || 'null') +} + describe('inspectedPageFlags read/write against page localStorage', () => { beforeEach(() => { // Karma runs in a real browser, so evalInWindow's code can be evaluated directly @@ -29,10 +47,12 @@ describe('inspectedPageFlags read/write against page localStorage', () => { } localStorage.removeItem(OVERRIDES_KEY) localStorage.removeItem(DEVTOOLS_MARKER_KEY) + clearSiteStores() registerCleanupTask(() => { ;(globalThis as any).chrome = previousChrome localStorage.removeItem(OVERRIDES_KEY) localStorage.removeItem(DEVTOOLS_MARKER_KEY) + clearSiteStores() }) }) @@ -109,4 +129,79 @@ describe('inspectedPageFlags read/write against page localStorage', () => { expect(Object.prototype.hasOwnProperty.call(storedValue, '__proto__')).toBe(true) expect(storedValue['__proto__']).toEqual({ enabled: true }) }) + + describe('site scoping', () => { + const override = { type: 'BOOLEAN', value: true } as const + + it("keeps a site's overrides in its own store and mirrors them into the key the wrapper reads", async () => { + await writeOverride('dark-mode', override, STAGING) + + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + expect(stored(OVERRIDES_KEY)).toEqual({ 'dark-mode': override }) + }) + + it('reads only the connected site when given one', async () => { + await writeOverride('dark-mode', override, STAGING) + await syncSiteOverrides(US1) + + expect((await readFlagState(STAGING))?.overrides).toEqual({ 'dark-mode': override }) + expect((await readFlagState(US1))?.overrides).toEqual({}) + }) + + it('reports every stored override when signed out, even one the page has not reloaded away from', async () => { + await writeOverride('dark-mode', override, STAGING) + // Switching empties the projection, but the page keeps applying staging's until it reloads — + // so the signed-out notice must still offer to clear it. + await syncSiteOverrides(US1) + + expect((await readFlagState())?.overrides).toEqual({ 'dark-mode': override }) + }) + + it("stops one site's override from applying on another", async () => { + await writeOverride('dark-mode', override, STAGING) + + const result = await syncSiteOverrides(US1) + + // The page would otherwise still be running staging's value under US1. + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(result?.changed).toBe(true) + // Staging keeps its own copy, so switching back restores it. + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + }) + + it('reports no change when the page is already running this site, so no reload is demanded', async () => { + await writeOverride('dark-mode', override, STAGING) + expect((await syncSiteOverrides(STAGING))?.changed).toBe(false) + }) + + it('drops overrides left in the wrapper key by a build without per-site stores', async () => { + localStorage.setItem(OVERRIDES_KEY, JSON.stringify({ legacy: override })) + + const result = await syncSiteOverrides(STAGING) + + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(result?.changed).toBe(true) + }) + + it("clearing while connected leaves the other sites' overrides alone", async () => { + await writeOverride('dark-mode', override, STAGING) + await writeOverride('other-flag', override, US1) + + await clearAllOverrides(US1) + + expect(stored(siteOverridesKey(US1))).toEqual({}) + expect(stored(siteOverridesKey(STAGING))).toEqual({ 'dark-mode': override }) + }) + + it('clearing while signed out wipes every store, so nothing reappears on reconnect', async () => { + await writeOverride('dark-mode', override, STAGING) + await writeOverride('other-flag', override, US1) + + await clearAllOverrides() + + expect(stored(OVERRIDES_KEY)).toEqual({}) + expect(localStorage.getItem(siteOverridesKey(STAGING))).toBeNull() + expect(localStorage.getItem(siteOverridesKey(US1))).toBeNull() + }) + }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index 3bc0b74a3c..50c7e1f63a 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -10,6 +10,17 @@ const logger = createLogger('inspectedPageFlags') export const OVERRIDES_KEY = 'dd.dd_flag.overrides' export const DEVTOOLS_MARKER_KEY = 'dd.dd_flag.devtools' +/** + * Per-site stores. The wrapper only reads OVERRIDES_KEY, so we keep that as a copy of the connected + * site's store and leave the other sites here, where the wrapper can't see them. The trailing dot + * stops these keys matching OVERRIDES_KEY itself. + */ +const SITE_OVERRIDES_PREFIX = `${OVERRIDES_KEY}.` + +export function siteOverridesKey(site: string): string { + return SITE_OVERRIDES_PREFIX + site +} + export interface FlagOverride { type: FlagType /** @@ -55,19 +66,27 @@ export function sanitizeOverrides(overrides: Record): FlagOverr return sanitized } -// Shared prelude for every inspected-window eval: parses the overrides map from localStorage, -// tolerating malformed/absent/mistyped storage, and leaves a normalized `overrides` in scope. -// Defined once so the read and mutation paths can't interpret storage differently as the contract -// evolves. -const READ_OVERRIDES_PRELUDE = ` - let overrides = {} - try { - const parsed = JSON.parse(localStorage.getItem(${JSON.stringify(OVERRIDES_KEY)}) || '{}') - if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - overrides = parsed +// Shared by every eval so all paths read storage the same way. `stable` ignores key order, so two +// equal maps compare equal. +const EVAL_HELPERS = ` + const parse = (raw) => { + try { + const parsed = JSON.parse(raw || 'null') + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null + } catch (e) { + return null } - } catch (e) {} + } + const stable = (map) => JSON.stringify(Object.keys(map).sort().map((k) => [k, map[k]])) +` + +// Leaves a mutable `overrides` in scope for the read and write paths. +function overridesPrelude(storeKey: string): string { + return ` + ${EVAL_HELPERS} + let overrides = parse(localStorage.getItem(${JSON.stringify(storeKey)})) || {} ` +} /** * Reads the current overrides and enablement marker straight from the inspected page's localStorage @@ -77,10 +96,29 @@ const READ_OVERRIDES_PRELUDE = ` * so the caller keeps its last good values instead of blanking the overrides and flashing the * "not detected" warning. */ -export async function readFlagState(): Promise { +export async function readFlagState(site?: string): Promise { + // Signed out we can't tell what the page is applying: a site switch repoints the projection before + // the reload that picks it up, so the page can still be running the site it loaded with. Report + // everything stored instead — which is also exactly what Clear all wipes there. + const source = site + ? overridesPrelude(siteOverridesKey(site)) + : ` + ${overridesPrelude(OVERRIDES_KEY)} + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i) + if (key && key.indexOf(${JSON.stringify(SITE_OVERRIDES_PREFIX)}) === 0) { + const store = parse(localStorage.getItem(key)) || {} + for (const flagKey of Object.keys(store)) { + if (!Object.prototype.hasOwnProperty.call(overrides, flagKey)) { + overrides[flagKey] = store[flagKey] + } + } + } + } + ` try { const raw = (await evalInWindow(` - ${READ_OVERRIDES_PRELUDE} + ${source} const devtoolsEnabled = localStorage.getItem(${JSON.stringify(DEVTOOLS_MARKER_KEY)}) === 'enabled' return { overrides, devtoolsEnabled } `)) as FlagState @@ -91,34 +129,83 @@ export async function readFlagState(): Promise { } } +/** + * Copies `site`'s store into the key the wrapper reads, so only that site's overrides apply. + * `changed` is false when it already matched, so callers don't ask for a needless reload. + * + * The store is the only source: anything written straight to OVERRIDES_KEY is overwritten, including + * overrides left by builds that predate per-site scoping. Those are dropped on first connect rather + * than migrated — the tab is days old, so there's nothing worth carrying forward. + * + * Null means the write failed — callers must surface that, since the page then keeps applying + * whichever site's overrides it already had. + */ +export async function syncSiteOverrides(site: string): Promise<{ changed: boolean } | null> { + const storeKey = siteOverridesKey(site) + try { + return (await evalInWindow(` + ${EVAL_HELPERS} + const projection = parse(localStorage.getItem(${JSON.stringify(OVERRIDES_KEY)})) || {} + const siteOverrides = parse(localStorage.getItem(${JSON.stringify(storeKey)})) || {} + const changed = stable(projection) !== stable(siteOverrides) + if (changed) { + localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, JSON.stringify(siteOverrides)) + } + return { changed } + `)) as { changed: boolean } + } catch (error) { + logger.error('Error while scoping flag overrides to the site:', error) + return null + } +} + /** * Reads, mutates, and writes back the overrides map in a single round trip, returning the resulting * map so the caller needs no follow-up read. Kept as one eval because splitting it would let a page * navigation land between read and write, applying the previous origin's overrides to the new one. */ -async function applyOverrideStatement(statement: string): Promise> { +async function applyOverrideStatement(statement: string, site?: string): Promise> { + const storeKey = site ? siteOverridesKey(site) : OVERRIDES_KEY return (await evalInWindow(` - ${READ_OVERRIDES_PRELUDE} + ${overridesPrelude(storeKey)} ${statement} - localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, JSON.stringify(overrides)) + const serialized = JSON.stringify(overrides) + localStorage.setItem(${JSON.stringify(storeKey)}, serialized) + ${site ? `localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, serialized)` : ''} return overrides `)) as Record } -export function writeOverride(key: string, override: FlagOverride): Promise> { +export function writeOverride(key: string, override: FlagOverride, site?: string): Promise> { // Parse the override from JSON *data* rather than interpolating an object literal, so a value // property named "__proto__" stays real data instead of the prototype setter (which would silently // drop it and persist {}). const overrideJson = JSON.stringify(JSON.stringify(override)) - return applyOverrideStatement(`overrides[${JSON.stringify(key)}] = JSON.parse(${overrideJson})`) + return applyOverrideStatement(`overrides[${JSON.stringify(key)}] = JSON.parse(${overrideJson})`, site) } -export function deleteOverride(key: string): Promise> { - return applyOverrideStatement(`delete overrides[${JSON.stringify(key)}]`) +export function deleteOverride(key: string, site?: string): Promise> { + return applyOverrideStatement(`delete overrides[${JSON.stringify(key)}]`, site) } -export function clearAllOverrides(): Promise> { - return applyOverrideStatement('overrides = {}') +/** + * Connected, clears the current site only. Signed out, clears every site — leaving the other stores + * would bring their overrides back on reconnect, as if the button hadn't worked. + */ +export async function clearAllOverrides(site?: string): Promise> { + if (site) { + return applyOverrideStatement('overrides = {}', site) + } + await evalInWindow(` + for (let i = localStorage.length - 1; i >= 0; i--) { + const key = localStorage.key(i) + if (key && key.indexOf(${JSON.stringify(SITE_OVERRIDES_PREFIX)}) === 0) { + localStorage.removeItem(key) + } + } + localStorage.setItem(${JSON.stringify(OVERRIDES_KEY)}, '{}') + `) + return {} } /** diff --git a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts index fbff50fc5c..6eef5dfd8d 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/oauth.spec.ts @@ -2,6 +2,7 @@ import { registerCleanupTask, replaceMockable } from '../../../../../../packages import { clearStoredTokens, getFlagsApiHost, + siteShortLabel, getValidAccessToken, loadStoredTokens, loginWithOAuth, @@ -37,11 +38,22 @@ describe('oauth', () => { } describe('getFlagsApiHost', () => { - it('maps each site to its frontend host (US1 → app, staging → dd)', () => { + it('maps each site to its frontend host (US1/EU1 → app, regional → own subdomain, staging → dd)', () => { expect(getFlagsApiHost('datadoghq.com')).toBe('app.datadoghq.com') + expect(getFlagsApiHost('datadoghq.eu')).toBe('app.datadoghq.eu') + expect(getFlagsApiHost('us3.datadoghq.com')).toBe('us3.datadoghq.com') + expect(getFlagsApiHost('us5.datadoghq.com')).toBe('us5.datadoghq.com') + expect(getFlagsApiHost('ap1.datadoghq.com')).toBe('ap1.datadoghq.com') + expect(getFlagsApiHost('ap2.datadoghq.com')).toBe('ap2.datadoghq.com') expect(getFlagsApiHost('datad0g.com')).toBe('dd.datad0g.com') }) + it('shortens a site label to its name, falling back to the raw site when unknown', () => { + expect(siteShortLabel('datadoghq.com')).toBe('US1') + expect(siteShortLabel('datad0g.com')).toBe('Staging') + expect(siteShortLabel('unknown.example')).toBe('unknown.example') + }) + it('throws on a site that is not in the known list', () => { expect(() => getFlagsApiHost('evil.example')).toThrowError(/Unknown Datadog site/) expect(() => getFlagsApiHost('')).toThrowError(/Unknown Datadog site/) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts b/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts index 8cb78da096..e0b481b666 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/oauth.ts @@ -46,14 +46,33 @@ export interface FlagSite { * The Datadog sites the Flags tab can connect to. A fixed list rather than a free-text host, so * there's no user-entered domain to validate against phishing. * - * Trimmed to US1 + Staging while the prod OAuth client replicates to the other DCs; add them back - * with the same subdomain scheme (US1/EU1 `app.`, staging `dd.`, regional sites as-is). + * Hosts follow the standard scheme: US1 and EU1 are served from `app.`, regional sites from their + * own subdomain, staging from `dd.`. Every non-staging site shares one prod OAuth client (see + * getClientId), which is replicated across the commercial DCs. + * + * FED (`ddog-gov.com`, `us2.ddog-gov.com`) is deliberately absent — GovCloud is a separate + * deployment and the client isn't registered there. Offering a site whose sign-in always fails is + * worse than omitting it. */ export const FLAG_SITES: FlagSite[] = [ { site: 'datadoghq.com', host: 'app.datadoghq.com', label: 'US1 (datadoghq.com)' }, + { site: 'us3.datadoghq.com', host: 'us3.datadoghq.com', label: 'US3 (us3.datadoghq.com)' }, + { site: 'us5.datadoghq.com', host: 'us5.datadoghq.com', label: 'US5 (us5.datadoghq.com)' }, + { site: 'datadoghq.eu', host: 'app.datadoghq.eu', label: 'EU1 (datadoghq.eu)' }, + { site: 'ap1.datadoghq.com', host: 'ap1.datadoghq.com', label: 'AP1 (ap1.datadoghq.com)' }, + { site: 'ap2.datadoghq.com', host: 'ap2.datadoghq.com', label: 'AP2 (ap2.datadoghq.com)' }, { site: 'datad0g.com', host: 'dd.datad0g.com', label: 'Staging (datad0g.com)' }, ] +/** + * The site's name without its host — "US1" rather than "US1 (datadoghq.com)" — for copy with no room + * for the full label. Falls back to the whole label, so a future entry without the host still reads. + */ +export function siteShortLabel(site: string): string { + const label = FLAG_SITES.find((entry) => entry.site === site)?.label ?? site + return label.split(' (')[0] +} + /** * Returns the frontend host serving OAuth + FFE for a site. Throws on an unknown site: the UI only * ever passes a value from FLAG_SITES, so an unknown one means a stale or hand-edited setting. diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts index b3b1ca7afa..bac54e3be3 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useInspectedPageOverrides.ts @@ -6,6 +6,7 @@ import { readFlagState, reloadInspectedPage, sanitizeOverrides, + syncSiteOverrides, writeOverride, } from './inspectedPageFlags' @@ -30,6 +31,10 @@ export interface OverridesController extends FlagPageState { clearOverride: (flagKey: string) => Promise clearAll: () => Promise reloadPage: () => void + /** Scoping changed which overrides apply; the page needs a reload to pick it up. */ + siteSwitchNeedsReload: boolean + /** Scoping failed, so the page may be applying another site's overrides. */ + scopeError: string | null } function delay(ms: number): Promise { @@ -46,11 +51,12 @@ function delay(ms: number): Promise { // - no read ever succeeded -> error async function settleFlagState( setState: Dispatch>, - isCancelled: () => boolean + isCancelled: () => boolean, + site?: string ): Promise { let lastRead: FlagState | null = null for (let attempts = 1; ; attempts++) { - const next = await readFlagState() + const next = await readFlagState(site) if (isCancelled()) { return } @@ -102,14 +108,19 @@ async function settleFlagState( * before deciding it's absent. * * Assumes a single mounted instance — the mutation queue only serializes writes within one hook. + * + * `site` scopes everything to the connected Datadog site, so overrides made on one neither apply nor + * show up on another. Omitted when signed out, where the caller wants what the page is applying. */ -export function useInspectedPageOverrides(): OverridesController { +export function useInspectedPageOverrides(site?: string): OverridesController { const [state, setState] = useState({ status: 'loading', overrides: {}, devtoolsEnabled: false, error: null, }) + const [siteSwitchNeedsReload, setSiteSwitchNeedsReload] = useState(false) + const [scopeError, setScopeError] = useState(null) // Serializes mutations so overlapping read-modify-writes can't clobber each other. const mutationQueue = useRef>(Promise.resolve()) // Cancels an in-flight settle when a newer navigation (or unmount) supersedes it. @@ -131,8 +142,35 @@ export function useInspectedPageOverrides(): OverridesController { cancelSettle.current = () => { cancelled = true } - void settleFlagState(setState, () => cancelled) - }, []) + void settleFlagState(setState, () => cancelled, site) + }, [site]) + + // Scope the page to the connected site. Reruns on navigation too, via `status` returning to ready. + // + // Known limitation (accepted): this runs outside the mutation queue, which covers one hook + // instance anyway — the provider remounts on a site change. A write sent just before the switch + // can land after this sync and restore the old site's copy until the next one. + useEffect(() => { + if (!site || state.status !== 'ready') { + return + } + let cancelled = false + void syncSiteOverrides(site).then((result) => { + if (cancelled) { + return + } + if (!result) { + setScopeError("Couldn't scope overrides to this site. The page may still be applying another site's.") + return + } + setScopeError(null) + // Prompting when nothing changed would make every sign-in demand a needless reload. + setSiteSwitchNeedsReload(result.changed) + }) + return () => { + cancelled = true + } + }, [site, state.status]) // Known limitation (accepted): terminal events aren't correlated to a specific navigation — the // webNavigation API exposes no id spanning onBeforeNavigate→onCompleted. In a rare overlapping- @@ -154,6 +192,8 @@ export function useInspectedPageOverrides(): OverridesController { // (before the re-render mirrors statusRef from state). readSeq.current += 1 statusRef.current = 'loading' + // This may be the reload the banner asked for; the sync after settling re-raises it if not. + setSiteSwitchNeedsReload(false) setState((prev) => ({ ...prev, status: 'loading', error: null })) } const onNavigationSettled = (details: { tabId: number; frameId: number }) => { @@ -201,13 +241,13 @@ export function useInspectedPageOverrides(): OverridesController { }, []) const setOverride = useCallback( - (flagKey: string, override: FlagOverride) => enqueue(() => writeOverride(flagKey, override)), - [enqueue] + (flagKey: string, override: FlagOverride) => enqueue(() => writeOverride(flagKey, override, site)), + [enqueue, site] ) - const clearOverride = useCallback((flagKey: string) => enqueue(() => deleteOverride(flagKey)), [enqueue]) + const clearOverride = useCallback((flagKey: string) => enqueue(() => deleteOverride(flagKey, site)), [enqueue, site]) - const clearAll = useCallback(() => enqueue(() => clearAllOverrides()), [enqueue]) + const clearAll = useCallback(() => enqueue(() => clearAllOverrides(site)), [enqueue, site]) const reloadPage = useCallback(() => reloadInspectedPage(), []) @@ -220,6 +260,8 @@ export function useInspectedPageOverrides(): OverridesController { clearOverride, clearAll, reloadPage, + siteSwitchNeedsReload, + scopeError, } } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts index 48cb09932c..648f039ec6 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.spec.ts @@ -1,9 +1,8 @@ import React, { act } from 'react' import { createRoot } from 'react-dom/client' import { registerCleanupTask } from '../../../../../../packages/browser-core/test' -import type { CatalogFlag } from './flagsRequests' import type { FlagAuthState } from './useFlagAuth' -import { useOverriddenFlags } from './useOverriddenFlags' +import { useOverriddenFlags, type OverriddenFlagsState } from './useOverriddenFlags' // useOverriddenFlags only reads isConnected + site off the auth object. const AUTH = { isConnected: true, site: 'datad0g.com' } as FlagAuthState @@ -28,7 +27,7 @@ describe('useOverriddenFlags', () => { function mountHook(keys: string[]) { const container = document.createElement('div') const root = createRoot(container) - let latest: CatalogFlag[] = [] + let latest: OverriddenFlagsState = { flags: [], missingKeys: new Set() } function Probe() { latest = useOverriddenFlags(AUTH, keys) return null @@ -57,7 +56,22 @@ describe('useOverriddenFlags', () => { await flush() expect(fetchSpy).toHaveBeenCalledTimes(2) - expect(get().map((flag) => flag.key)).toEqual(jasmine.arrayWithExactContents(['flag-a', 'flag-b'])) + expect(get().flags.map((flag) => flag.key)).toEqual(jasmine.arrayWithExactContents(['flag-a', 'flag-b'])) + expect(get().missingKeys.size).toBe(0) + }) + + it('reports a key the catalog has no match for as missing', async () => { + spyOn(globalThis, 'fetch').and.callFake((input: RequestInfo | URL) => { + const key = new URL(input as string).searchParams.get('key') + const data = key === 'gone' ? [] : [{ attributes: { key, name: `Name ${key}`, value_type: 'STRING' } }] + return Promise.resolve(new Response(JSON.stringify({ data }))) + }) + + const get = mountHook(['flag-a', 'gone']) + await flush() + + expect(get().flags.map((flag) => flag.key)).toEqual(['flag-a']) + expect(Array.from(get().missingKeys)).toEqual(['gone']) }) it('does not fetch when there are no overridden keys', async () => { @@ -65,18 +79,16 @@ describe('useOverriddenFlags', () => { const get = mountHook([]) await flush() expect(fetchSpy).not.toHaveBeenCalled() - expect(get()).toEqual([]) + expect(get().flags).toEqual([]) }) - it('returns no flags when a key lookup fails (non-blocking)', async () => { - // The failure is expected here; the hook logs it via console.error — swallow it so the CI - // unexpected-error-log reporter doesn't flag the intentional log. - spyOn(console, 'error') + it('does not report a key as missing when its lookup failed', async () => { spyOn(globalThis, 'fetch').and.returnValue( Promise.resolve(new Response('nope', { status: 500, statusText: 'Server Error' })) ) const get = mountHook(['flag-a']) await flush() - expect(get()).toEqual([]) + expect(get().flags).toEqual([]) + expect(get().missingKeys.size).toBe(0) }) }) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts index 8e83733315..b80e3048a3 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/useOverriddenFlags.ts @@ -7,15 +7,24 @@ import type { FlagAuthState } from './useFlagAuth' const logger = createLogger('useOverriddenFlags') +export interface OverriddenFlagsState { + flags: CatalogFlag[] + /** Keys a well-formed lookup proved absent. Empty while loading and after a failure. */ + missingKeys: ReadonlySet +} + +const EMPTY: OverriddenFlagsState = { flags: [], missingKeys: new Set() } + /** * Loads the catalog data (name, variants, type) for the currently-overridden flag keys, so the - * "Local overrides" section can render them even when they're not on the current catalog page. Keys - * that don't resolve to a flag are simply absent — the caller falls back to a minimal row so every - * override can still be reverted. Failures are non-blocking (the section just shows fallback rows). + * "Local overrides" section can render them even when they're not on the current catalog page. A key + * that doesn't resolve falls back to a minimal row so the override can still be reverted, and lands + * in `missingKeys`, which the row notes as archived or deleted. Failures are non-blocking: the + * section shows fallback rows, unmarked. */ -export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): CatalogFlag[] { +export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): OverriddenFlagsState { const { isConnected, site } = auth - const [flags, setFlags] = useState([]) + const [state, setState] = useState(EMPTY) // Sort + join so the effect only reruns when the *set* of keys changes, not on every render (the // caller passes a fresh array each time). @@ -24,22 +33,24 @@ export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): Catalog useEffect(() => { const keyArray = keyList ? keyList.split('\n') : [] if (!isConnected || keyArray.length === 0) { - setFlags([]) + setState(EMPTY) return } let cancelled = false + // Drop the previous verdict while the new lookup runs; keep the flags so rows don't blank out. + setState((previous) => ({ flags: previous.flags, missingKeys: new Set() })) getValidAccessToken(site) - .then((token) => (token ? fetchFlagsByKeys(token, site, keyArray) : [])) - .then((loaded) => { + .then((token) => (token ? fetchFlagsByKeys(token, site, keyArray) : { flags: [], missingKeys: [] })) + .then(({ flags, missingKeys }) => { if (!cancelled) { - setFlags(loaded) + setState({ flags, missingKeys: new Set(missingKeys) }) } }) .catch((err: unknown) => { if (!cancelled) { logger.error('Error while fetching overridden flags:', err) - setFlags([]) + setState(EMPTY) } }) @@ -48,5 +59,5 @@ export function useOverriddenFlags(auth: FlagAuthState, keys: string[]): Catalog } }, [isConnected, site, keyList]) - return flags + return state }