🐛 address Codex review feedback

Author: mormubis

packages/browser-core/src/domain/remoteConfiguration/remoteConfigurationFetch.ts

@@ -1,4 +1,5 @@
 import { buildEndpointUrl } from '@datadog/js-core/transport'
+import { globalObject } from '@datadog/js-core/util'
 import { fetch } from '../../browser/fetch'
 import type { RumSdkConfig } from './remoteConfiguration.types'
 
@@ -15,6 +16,22 @@ export interface RemoteConfigurationEndpointOptions {
 
 export type FetchRemoteConfigurationResult = { ok: true; value: RemoteConfiguration } | { ok: false; error: Error }
 
+// Typed interface for the global inflight fetch registry so deduplication
+// works across separate SDK bundles (e.g. RUM and Logs loaded as separate CDN
+// scripts on the same page) and in service-worker environments where `window`
+// is not available.
+interface GlobalWithInflightFetches {
+  __ddRcInflight?: Map<string, Promise<FetchRemoteConfigurationResult>>
+}
+
+function getInflightFetches(): Map<string, Promise<FetchRemoteConfigurationResult>> {
+  const global = globalObject as GlobalWithInflightFetches
+  if (!global.__ddRcInflight) {
+    global.__ddRcInflight = new Map()
+  }
+  return global.__ddRcInflight
+}
+
 export function getRemoteConfigurationId(options: RemoteConfigurationEndpointOptions): string | undefined {
   return options.remoteConfiguration?.id ?? options.remoteConfigurationId
 }
@@ -31,30 +48,17 @@ export function buildEndpoint(options: RemoteConfigurationEndpointOptions): stri
   })
 }
 
-// Use a window-level registry so deduplication works across separate SDK bundles
-// (e.g. RUM and Logs loaded as separate CDN scripts on the same page).
-const INFLIGHT_FETCHES_KEY = '__ddRcInflight'
-
-function getInflightFetches(): Map<string, Promise<FetchRemoteConfigurationResult>> {
-  const win = window as unknown as Record<string, unknown>
-  if (!win[INFLIGHT_FETCHES_KEY]) {
-    win[INFLIGHT_FETCHES_KEY] = new Map<string, Promise<FetchRemoteConfigurationResult>>()
-  }
-  return win[INFLIGHT_FETCHES_KEY] as Map<string, Promise<FetchRemoteConfigurationResult>>
-}
-
 export function fetchRemoteConfiguration(
   options: RemoteConfigurationEndpointOptions
 ): Promise<FetchRemoteConfigurationResult> {
   const endpoint = buildEndpoint(options)
   const inflightFetches = getInflightFetches()
 
   if (!inflightFetches.has(endpoint)) {
-    const win = window as unknown as Record<string, unknown>
     const promise = doFetchRemoteConfiguration(endpoint).finally(() => {
       inflightFetches.delete(endpoint)
       if (inflightFetches.size === 0) {
-        delete win[INFLIGHT_FETCHES_KEY]
+        delete (globalObject as GlobalWithInflightFetches).__ddRcInflight
       }
     })
     inflightFetches.set(endpoint, promise)

packages/browser-logs/src/boot/preStartLogs.spec.ts

@@ -9,6 +9,7 @@ import {
   replaceMockable,
   replaceMockableWithSpy,
   createStartSessionManagerMock,
+  registerCleanupTask,
 } from '@datadog/browser-core/test'
 import type { TrackingConsentState } from '@datadog/browser-core'
 import {
@@ -258,10 +259,6 @@ describe('preStartLogs', () => {
   describe('remote configuration', () => {
     const RC_ID = 'test-rc-id'
 
-    afterEach(() => {
-      localStorage.removeItem(buildCacheKey(RC_ID))
-    })
-
     it('applies cached remote config overrides before starting', async () => {
       localStorage.setItem(
         buildCacheKey(RC_ID),
@@ -271,6 +268,7 @@ describe('preStartLogs', () => {
           fetchedAt: Date.now(),
         })
       )
+      registerCleanupTask(() => localStorage.removeItem(buildCacheKey(RC_ID)))
 
       const { strategy, doStartLogsSpy } = createPreStartStrategyWithDefaults()
       strategy.init({ clientToken: 'xxx', remoteConfiguration: { id: RC_ID } })

packages/browser-logs/src/domain/configuration.ts

@@ -74,14 +74,18 @@ export interface LogsInitConfiguration extends InitConfiguration {
   forwardReports?: RawReportType[] | 'all' | undefined
 
   /**
-   * The ID of the remote configuration to apply. Use this for the non-blocking cache-and-reload path.
+   * The ID of the remote configuration to apply. Triggers synchronous loading — the SDK waits for
+   * the fetch to complete before starting. Use `remoteConfiguration: { id }` for the non-blocking
+   * cache-and-reload path instead.
    *
    * @category Remote Configuration
    */
   remoteConfigurationId?: string | undefined
 
   /**
-   * Remote configuration options.
+   * Remote configuration options. Use `{ id }` for the non-blocking cache-and-reload path (the SDK
+   * starts immediately using a cached value and refreshes in the background). Use `{ id, sync: true }`
+   * to block startup on a live fetch.
    *
    * @category Remote Configuration
    */

packages/browser-logs/src/domain/remoteConfiguration.spec.ts

@@ -1,5 +1,5 @@
 import { CACHE_VERSION, buildCacheKey, display } from '@datadog/browser-core'
-import { interceptRequests } from '@datadog/browser-core/test'
+import { interceptRequests, registerCleanupTask } from '@datadog/browser-core/test'
 import type { LogsInitConfiguration } from './configuration'
 import {
   applyLogsRemoteConfiguration,
@@ -62,8 +62,9 @@ describe('getLogsRemoteConfiguration', () => {
     remoteConfiguration: { id: RC_ID },
   }
 
-  afterEach(() => {
-    localStorage.removeItem(buildCacheKey(RC_ID))
+  beforeEach(() => {
+    // Prevent background sync from firing real network requests
+    interceptRequests()
   })
 
   it('returns the initConfiguration with remote overrides applied on cache hit', () => {
@@ -75,6 +76,7 @@ describe('getLogsRemoteConfiguration', () => {
         fetchedAt: Date.now(),
       })
     )
+    registerCleanupTask(() => localStorage.removeItem(buildCacheKey(RC_ID)))
 
     const result = getLogsRemoteConfiguration(initConfiguration)
     expect(result!.forwardErrorsToLogs).toBeFalse()

packages/browser-logs/src/domain/remoteConfiguration.ts

@@ -43,7 +43,9 @@ export function getLogsRemoteConfiguration(
   // Background sync — update the cache for the next page load
   fetchRemoteConfiguration(initConfiguration)
     .then((fetchResult) => {
-      if (fetchResult.ok) {
+      if (!fetchResult.ok) {
+        display.error(fetchResult.error)
+      } else {
         cache.write(fetchResult.value)
       }
     })