diff --git a/developer-extension/src/common/toErrorMessage.ts b/developer-extension/src/common/toErrorMessage.ts new file mode 100644 index 0000000000..1ce86f218e --- /dev/null +++ b/developer-extension/src/common/toErrorMessage.ts @@ -0,0 +1,5 @@ +// Turns an unknown caught value into a displayable string: an Error's message, or the value coerced +// to a string. One place to change how errors read across the panel. +export function toErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} diff --git a/developer-extension/src/panel/components/panel.tsx b/developer-extension/src/panel/components/panel.tsx index 6656dd1f8e..467861ccd3 100644 --- a/developer-extension/src/panel/components/panel.tsx +++ b/developer-extension/src/panel/components/panel.tsx @@ -54,11 +54,9 @@ export function Panel() { Live replay - {settings.datadogMode && ( - - Feature Flags - - )} + + Feature Flags + - {settings.datadogMode && ( - - - - )} + + + diff --git a/developer-extension/src/panel/components/tabBase.module.css b/developer-extension/src/panel/components/tabBase.module.css index fbab67f9e1..f925e30595 100644 --- a/developer-extension/src/panel/components/tabBase.module.css +++ b/developer-extension/src/panel/components/tabBase.module.css @@ -4,6 +4,11 @@ .topContainer { margin: 0; + /* Sit above the scrolling content and cast a soft shadow onto it, so a long list reads as scrolling + *under* the header instead of looking cut off at the top edge. Applies to every tab's top bar. */ + position: relative; + z-index: 1; + box-shadow: 0 4px 8px -6px rgba(0, 0, 0, 0.25); } .leftContainer { diff --git a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx index 83228de118..2b630c8751 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/connectScreen.tsx @@ -1,18 +1,25 @@ -import { Anchor, Badge, Button, Center, Group, Select, Stack, Text } from '@mantine/core' -import React, { useState } from 'react' +import { Alert, Badge, Box, Button, Center, Group, Select, Stack, Text } from '@mantine/core' +import React from 'react' import { useSettings } from '../../../hooks/useSettings' import type { FlagAuthState } from './useFlagAuth' +import { useInspectedPageOverrides } from './useInspectedPageOverrides' import { FLAG_SITES } from './oauth' export function ConnectScreen({ auth }: { auth: FlagAuthState }) { - const [advancedOpen, setAdvancedOpen] = useState(false) - return (
+ Authenticate with Datadog to access your feature flags + {/* Pick the site before signing in: it selects which Datadog OAuth server + FFE API the flow + talks to (see FLAG_SITES), so it must be set before the Sign in button runs that flow. */} + + {/* Locked while signing in: the chosen site is baked into the OAuth flow already running, so + switching mid-flow would point the resulting token at a different environment. */} + + @@ -21,39 +28,78 @@ export function ConnectScreen({ auth }: { auth: FlagAuthState }) { {auth.error} )} - - setAdvancedOpen((open) => !open)}> - {advancedOpen ? '− Hide advanced' : 'Advanced: site'} - - {advancedOpen && ( - - - + {/* A revocation that failed leaves the grant live at Datadog while this panel is signed out, + so the notice belongs on this screen — it's the one the user lands on after disconnecting. */} + {auth.warning && ( + + {auth.warning} You can revoke it from Datadog under Organization Settings → Authorized Applications. + )}
) } +/** + * 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. + * + * 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 count = Object.keys(overrides).length + + if (status !== 'ready' || count === 0) { + return null + } + + return ( + // Masked despite the surrounding dd-privacy-allow: only a count renders today, but flag keys are + // customer data, so anything added here should stay out of the extension's own Session Replay. + + + These are stored in the page and keep applying while you are signed out. Sign in to view and remove them. + + + ) +} + export function ConnectionHeader({ auth }: { auth: FlagAuthState }) { return ( - - - - Connected via OAuth - - - {auth.site} - - - - {/* Surface disconnect failures here too — otherwise a failed Disconnect looks like a no-op. */} + {/* Surface disconnect failures here too — otherwise a failed Disconnect looks like a no-op. + (A revoke-succeeded-but-grant-live warning can't appear here: it always accompanies a + successful local sign-out, which flips to the ConnectScreen where the notice lives.) */} {auth.error && ( - + {auth.error} )} @@ -61,7 +107,13 @@ export function ConnectionHeader({ auth }: { auth: FlagAuthState }) { ) } -function SiteField() { +// Falls back to the raw site so a stale or hand-edited setting still renders something meaningful +// (getFlagsApiHost is the one that treats an unknown site as an error). +function siteLabel(site: string): string { + return FLAG_SITES.find((entry) => entry.site === site)?.label ?? site +} + +function SiteField({ disabled }: { disabled?: boolean }) { const [{ flagsSite }, setSetting] = useSettings() return ( @@ -72,6 +124,7 @@ function SiteField() { value={flagsSite} onChange={(value) => value && setSetting('flagsSite', value)} allowDeselect={false} + disabled={disabled} size="xs" /> ) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/ffeApi.ts b/developer-extension/src/panel/components/tabs/flagsTab/ffeApi.ts new file mode 100644 index 0000000000..14a4ed6b9f --- /dev/null +++ b/developer-extension/src/panel/components/tabs/flagsTab/ffeApi.ts @@ -0,0 +1,28 @@ +// Shared helper for the FFE API calls the Flags tab makes (catalog, current user, teams). Centralizes +// the bearer-auth header + response handling that flagsRequests.ts and flagIdentity.ts would +// otherwise each repeat. + +// Thrown on a 403 so callers can tell "the token lacks the scope" apart from a real failure (used by +// flagIdentity to degrade the team filter rather than fail the whole tab). +export class ForbiddenError extends Error {} + +/** + * GETs a JSON resource from the FFE API with the OAuth bearer token. Throws ForbiddenError on 403 and + * a generic Error on any other non-2xx, prefixing the message with `errorLabel`. Keep customer data + * (e.g. a flag key) out of `errorLabel` — these errors are logged and the panel forwards logs to its + * own telemetry. + */ +export async function fetchFfeJson(url: string, token: string, errorLabel: string): Promise { + const response = await fetch(url, { + headers: { + Authorization: `Bearer ${token}`, + }, + }) + if (response.status === 403) { + throw new ForbiddenError(`${errorLabel}: 403 ${response.statusText}`) + } + if (!response.ok) { + throw new Error(`${errorLabel}: ${response.status} ${response.statusText}`) + } + return (await response.json()) as T +} diff --git a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx index bae8e54626..99ece77997 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/flagCatalogList.tsx @@ -1,6 +1,19 @@ -import { ActionIcon, Box, Button, Code, CopyButton, Group, Loader, Space, Text, Tooltip } from '@mantine/core' +import { + ActionIcon, + Anchor, + Box, + Button, + Code, + CopyButton, + Group, + Loader, + Space, + Stack, + Text, + Tooltip, +} from '@mantine/core' import { IconArrowBackUp, IconCopy } from '@tabler/icons-react' -import React, { type ReactNode } from 'react' +import React, { useLayoutEffect, useRef, useState, type ReactNode } from 'react' import type { CatalogFlag } from './flagsRequests' import { useFlagsContext } from './flagsContext' import { validateOverrideValue } from './flagTypes' @@ -29,7 +42,7 @@ export function FlagCatalogBody() { - + ) } -// Renders a bordered list of flag rows, or `emptyMessage` when there are none. Shared by the catalog -// body and the "Local overrides" section — they differ only in border color and empty copy. Reads -// the override state + actions from context so each row's wiring stays identical. +/** + * A bordered list of flag rows, shared by the catalog body and the "Local overrides" section — they + * differ only in border color and empty copy. + */ function FlagList({ flags, borderColor, @@ -113,20 +126,22 @@ function FlagRow({ - + {flag.name} - + {flag.description && } + {overridden && ( @@ -142,10 +157,9 @@ function FlagRow({ ) : ( flag.variants.map((variant) => { const isActive = overridden && valuesEqual(override.value, variant.value) - // The catalog falls back to the raw string when a variant doesn't parse as its - // declared type (see parseVariantValue) — writing that through would violate the - // same contract validateOverrideValue enforces for manual overrides. `allowNull` keeps - // a legitimate JSON `null` variant applyable (a raw-string type mismatch still fails). + // 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. const validationError = validateOverrideValue(flag.type, variant.value, { allowNull: true }) return ( - - - setAddOpen((open) => !open)}> - {addOpen ? '− Hide custom override' : '+ Add a custom override'} - - {addOpen && ( - <> - - - - )} ) diff --git a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts index dc18aaff9d..3bc0b74a3c 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts +++ b/developer-extension/src/panel/components/tabs/flagsTab/inspectedPageFlags.ts @@ -4,16 +4,18 @@ import type { FlagType } from './flagTypes' const logger = createLogger('inspectedPageFlags') -// Contract shared with @datadog/openfeature-browser's DatadogDevtools wrapper. -// Keep these in sync with that package: the wrapper reads OVERRIDES_KEY once on -// initialize() and writes DEVTOOLS_MARKER_KEY when it is composed into the provider stack. +// Contract shared with @datadog/openfeature-browser's DatadogDevtools wrapper. Keep these in sync +// with that package: the wrapper reads OVERRIDES_KEY once on initialize() and writes +// DEVTOOLS_MARKER_KEY when it is composed into the provider stack. export const OVERRIDES_KEY = 'dd.dd_flag.overrides' export const DEVTOOLS_MARKER_KEY = 'dd.dd_flag.devtools' export interface FlagOverride { type: FlagType - // Any JSON value — objects/arrays are `object`; `null` is allowed (a flag/variant value can be - // null). The manual-entry form still rejects null via validateOverrideValue. + /** + * Any JSON value — objects/arrays are `object`, and `null` is allowed (a variant value can be + * null). The manual-entry form still rejects null via validateOverrideValue. + */ value: boolean | string | number | object | null } @@ -25,22 +27,24 @@ export interface FlagState { } /** - * Look up an override by key using own-property membership — a plain `overrides[key]` would - * return inherited members (e.g. Object.prototype.constructor) for flags named "constructor", - * "toString", etc., making them wrongly appear overridden. + * Looks up an override by own-property membership — a plain `overrides[key]` would return inherited + * members (e.g. Object.prototype.constructor) for flags named "constructor" or "toString", making + * them wrongly appear overridden. */ export function getOverride(overrides: FlagOverrides, key: string): FlagOverride | undefined { return Object.prototype.hasOwnProperty.call(overrides, key) ? overrides[key] : undefined } -// The page's localStorage isn't exclusively ours to write — a hand-edited value, or a different -// version of the DatadogDevtools wrapper, could leave an entry that isn't a FlagOverride shape -// (e.g. `null`). Drop those rather than let `override.value` crash the panel when rendered. A -// wrong-typed-but-shaped entry is kept so it stays visible and removable (the provider rejects it). function isFlagOverride(value: unknown): value is FlagOverride { return typeof value === 'object' && value !== null && 'type' in value && 'value' in value } +/** + * Drops entries that aren't FlagOverride-shaped. The page's localStorage isn't exclusively ours — a + * hand-edited value or a different wrapper version could leave something that would crash the panel + * on render. A wrong-typed-but-shaped entry is kept so it stays visible and removable (the provider + * rejects it anyway). + */ export function sanitizeOverrides(overrides: Record): FlagOverrides { const sanitized: FlagOverrides = {} for (const [key, entry] of Object.entries(overrides)) { @@ -51,10 +55,10 @@ export function sanitizeOverrides(overrides: Record): FlagOverr return sanitized } -// Shared read/normalize prelude for every inspected-window eval: parse the overrides map from -// localStorage and tolerate malformed/absent/mistyped storage, leaving a normalized `overrides` -// object in scope. Defined once so the read and mutation paths can't interpret storage differently -// as the DatadogDevtools contract evolves. +// 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 { @@ -66,8 +70,12 @@ const READ_OVERRIDES_PRELUDE = ` ` /** - * Reads the current overrides and enablement marker straight from the inspected page's - * localStorage. The page is the single source of truth — we never cache it elsewhere. + * Reads the current overrides and enablement marker straight from the inspected page's localStorage + * — the page is the single source of truth, never cached elsewhere. + * + * Returns null on a transient eval failure (the page navigating or busy) rather than an empty state, + * so the caller keeps its last good values instead of blanking the overrides and flashing the + * "not detected" warning. */ export async function readFlagState(): Promise { try { @@ -78,18 +86,16 @@ export async function readFlagState(): Promise { `)) as FlagState return { overrides: sanitizeOverrides(raw.overrides ?? {}), devtoolsEnabled: !!raw.devtoolsEnabled } } catch (error) { - // A transient eval failure (the inspected page navigating/reloading, or busy) is NOT the same as - // "no overrides and no wrapper". Return null so the caller keeps its last good state rather than - // blanking the overrides and flashing the "not detected" warning. logger.error('Error while reading flag overrides:', error) return null } } -// Reads, mutates, and writes back the overrides map in a single inspected-window round trip, then -// returns the resulting map so the caller can update its state without a second read. Splitting this -// into a separate read then write would let a page navigation land between the two, applying the -// previous origin's overrides on top of the new origin's storage. +/** + * 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> { return (await evalInWindow(` ${READ_OVERRIDES_PRELUDE} @@ -100,9 +106,9 @@ async function applyOverrideStatement(statement: string): Promise> { - // Parse the override from JSON *data* rather than interpolating it as an object literal, so a value - // property named "__proto__" stays real data instead of the object-literal prototype setter (which - // would silently drop it and persist {}). + // 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})`) } @@ -115,8 +121,10 @@ export function clearAllOverrides(): Promise> { return applyOverrideStatement('overrides = {}') } -// Reloads the inspected page so the DatadogDevtools wrapper re-reads localStorage and (re)applies the -// current overrides. Overrides are written immediately; this is only how they take effect. +/** + * Reloads the inspected page so the wrapper re-reads localStorage. Overrides are written + * immediately; this is only how they take effect. + */ export function reloadInspectedPage(): void { chrome.devtools.inspectedWindow.reload({}) } diff --git a/developer-extension/src/panel/components/tabs/flagsTab/manualOverrideForm.tsx b/developer-extension/src/panel/components/tabs/flagsTab/manualOverrideForm.tsx index 397412a366..564ef16861 100644 --- a/developer-extension/src/panel/components/tabs/flagsTab/manualOverrideForm.tsx +++ b/developer-extension/src/panel/components/tabs/flagsTab/manualOverrideForm.tsx @@ -1,5 +1,18 @@ -import { Box, Button, Group, JsonInput, SegmentedControl, Space, Stack, Switch, Text, TextInput } from '@mantine/core' +import { + Box, + Button, + Code, + Group, + JsonInput, + SegmentedControl, + Space, + Stack, + Switch, + Text, + TextInput, +} from '@mantine/core' import React, { useState } from 'react' +import { toErrorMessage } from '../../../../common/toErrorMessage' import { useFlagsContext } from './flagsContext' import { FLAG_TYPES, @@ -9,26 +22,50 @@ import { validateOverrideValue, type FlagType, } from './flagTypes' -import type { FlagOverride } from './inspectedPageFlags' +import { getOverride, type FlagOverride } from './inspectedPageFlags' export function ManualOverrideForm() { - const { catalog, applyOverride } = useFlagsContext() + const { catalog, overrides, applyOverride, mutationError } = useFlagsContext() const [flagKey, setFlagKey] = useState('') const [type, setType] = useState('BOOLEAN') const [booleanValue, setBooleanValue] = useState(true) const [textValue, setTextValue] = useState('') const [error, setError] = useState(null) + // The last override submitted, used to report the outcome inline: the form sits below a long + // catalog, so both the tab-level alert and the "Local overrides" section are easily offscreen from + // here, and without this Apply looks like it did nothing. + const [submitted, setSubmitted] = useState<{ key: string; override: FlagOverride } | null>(null) + + const trimmedKey = flagKey.trim() + const existingOverride = getOverride(overrides, trimmedKey) + // Derived from the stored overrides rather than the applyOverride call, so it reflects what the + // page actually holds. A write failure leaves this false and surfaces mutationError instead. + // Type is compared alongside value: re-submitting a stored INTEGER 1 as NUMERIC would otherwise + // look already-applied and report success for a write that never landed. + const stored = submitted && getOverride(overrides, submitted.key) + const applied = + submitted !== null && + stored?.type === submitted.override.type && + valuesEqual(stored?.value, submitted.override.value) + + /** Wraps a setter so any edit drops the previous outcome, rather than leaving it next to changed input. */ + function edit(set: (value: T) => void) { + return (value: T) => { + setSubmitted(null) + setError(null) + set(value) + } + } function submit() { setError(null) - const trimmedKey = flagKey.trim() + setSubmitted(null) if (!trimmedKey) { setError('Flag key is required') return } - // If the key matches a flag on the loaded page, catch a type mismatch early — the provider would - // otherwise silently reject it at resolve time. (A flag not on the current page is treated as - // unknown here; the provider still rejects a true mismatch.) + // Catch a type mismatch early — the provider would otherwise silently reject it at resolve time. + // A flag not on the current page is treated as unknown; the provider still rejects a true mismatch. const catalogFlag = catalog.flags.find((flag) => flag.key === trimmedKey) if (catalogFlag && catalogFlag.type !== type) { setError(`"${trimmedKey}" is a ${flagTypeLabel(catalogFlag.type)} flag in the catalog — use that type instead`) @@ -38,7 +75,7 @@ export function ManualOverrideForm() { try { value = parseFormValue(type, type === 'BOOLEAN' ? booleanValue : textValue) } catch (err) { - setError(err instanceof Error ? err.message : String(err)) + setError(toErrorMessage(err)) return } const validationError = validateOverrideValue(type, value) @@ -47,6 +84,7 @@ export function ManualOverrideForm() { return } applyOverride(trimmedKey, { type, value }) + setSubmitted({ key: trimmedKey, override: { type, value } }) setError(null) } @@ -56,8 +94,11 @@ export function ManualOverrideForm() { label="Flag key" placeholder="my-flag" value={flagKey} - onChange={(event) => setFlagKey(event.currentTarget.value)} + onChange={(event) => edit(setFlagKey)(event.currentTarget.value)} size="xs" + // Re-applying an existing key replaces its value rather than failing, so say so up front + // instead of letting the change look like a no-op. + description={existingOverride ? 'Already overridden — applying replaces the current value.' : undefined} /> @@ -68,7 +109,7 @@ export function ManualOverrideForm() { color="violet" size="xs" value={type} - onChange={(value) => setType(value)} + onChange={(value) => edit(setType)(value)} data={FLAG_TYPES.map((flagType) => ({ value: flagType, label: flagType }))} /> @@ -77,17 +118,24 @@ export function ManualOverrideForm() { setBooleanValue(event.currentTarget.checked)} + onChange={(event) => edit(setBooleanValue)(event.currentTarget.checked)} color="violet" /> ) : type === 'JSON' ? ( - + ) : ( setTextValue(event.currentTarget.value)} + onChange={(event) => edit(setTextValue)(event.currentTarget.value)} size="xs" /> )} @@ -98,6 +146,21 @@ export function ManualOverrideForm() { )} + {/* Gated on `submitted` so a failed write from elsewhere in the tab (a catalog row) doesn't + light up this form — that one belongs to the tab-level alert. */} + {submitted !== null && + (applied ? ( + + Override set for {submitted.key} — refresh the page to apply it. + + ) : ( + mutationError && ( + + {mutationError} + + ) + ))} +