You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Importing @datadog/electron-sdk/instrument before electron but deferring init() (a common pattern when
gating on user consent) made the first window/frame load hang for ~30s. The injected preload does a
synchronous ipcRenderer.sendSync('datadog:bridge-config') at frame commit, but the responder was only
registered by init(). With no responder, Electron blocked the renderer until the sendSync timeout.
Register the datadog:bridge-config responder unconditionally at instrument time, so the preload's
synchronous request always resolves immediately, whether or not init() has run.
Back the responder with a process-global config holder seeded with safe fallback values
(mask privacy, no extra webview hosts). It must be a process global because the instrument and init() entry points ship in separate bundles.
init() (via RendererPipeline) now updates the holder with the real config instead of owning the
responder. A window loaded before init() reads the fallback; one loaded after reads the real config.
Document the lifecycle in ARCHITECTURE.md and the pre-init reload caveat in the README.
Test instructions
In a fresh Electron app, require('@datadog/electron-sdk/instrument') before electron and do NOT call init().
Open a BrowserWindow and load any content.
Observe the window loads promptly (previously it hung ~30s) with no sendSync ... without listeners warning.
Call init(...), open a second window, and confirm it picks up the configured allowedWebViewHosts / defaultPrivacyLevel.
Checklist
Tested locally (playground)
Added unit tests for this change.
Added e2e/integration tests for this change.
Updated related documentation.
Agentic code review findings addressed or explicitly dismissed.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
PR Review — Score: 4.7 / 5
This is a well-targeted fix for the deferred-init hang: registering the datadog:bridge-config responder at instrument time, backed by a process-global holder, directly addresses the root cause while preserving safe fallback behavior. The Symbol.for pattern matches existing instrumentation idempotency, monitor() wraps the IPC callback correctly, and the unit plus e2e coverage give good confidence. Documentation clearly explains the lifecycle and the pre-init reload caveat. I would approve this PR.
Why 4.7: Correct architectural split between instrument-time registration and init-time config publication; thorough tests (holder sharing, responder behavior, deferred-init e2e); docs updated in README and ARCHITECTURE.md; fallback values are conservative.
Why not 5: Minor robustness gap around returning a live config reference from the holder, and the new e2e scenario asserts host propagation but not defaultPrivacyLevel (unit tests cover the holder, so risk is low).
Findings
[Minor] Live config reference — getBridgeConfig() returns the holder's object directly; accidental mutation could affect subsequent IPC responses.
[Nit] E2e privacy assertion — deferred-init scenario verifies allowedWebViewHosts before/after init but not defaultPrivacyLevel.
Architectural flow
sequenceDiagram
participant App
participant Instrument
participant Holder as Process-global holder
participant IPC as ipcMain responder
participant Preload
participant Init as init() / RendererPipeline
App->>Instrument: import instrument (before electron)
Instrument->>IPC: register datadog:bridge-config listener
Instrument->>Holder: seed fallback (mask, [])
Note over App,Preload: Window opens before init()
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig()
Holder-->>IPC: fallback config
IPC-->>Preload: immediate response (no hang)
App->>Init: init(config) after consent gate
Init->>Holder: setBridgeConfig(real config)
Note over App,Preload: New window after init
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig()
Holder-->>IPC: real config
IPC-->>Preload: configured values
Loading
Before:RendererPipeline registered the datadog:bridge-config responder only during init(). When init() was deferred (e.g. user-consent gating), the preload's synchronous sendSync found no listener and blocked the renderer for ~30s.
After: The responder is registered unconditionally at instrument time and reads from a process-global holder seeded with safe fallbacks. init() publishes the real configuration via setBridgeConfig, so windows opened before init load promptly with fallback values and windows opened after init receive the configured privacy level and webview hosts.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
PR Review — Score: 4.9 / 5
This is a well-targeted fix for the deferred-init hang. Registering the datadog:bridge-config responder at instrument time, backed by a process-global holder, directly addresses the root cause while preserving safe fallback behavior. Follow-up commits since the prior review addressed both earlier findings (getBridgeConfig() now returns a defensive copy; the e2e scenario asserts defaultPrivacyLevel before and after init). The Symbol.for pattern matches existing instrumentation idempotency, monitor() wraps the IPC callback correctly, and the unit plus e2e coverage give strong confidence. Documentation clearly explains the lifecycle and the pre-init reload caveat. I would approve this PR.
Why 4.9: Correct architectural split between instrument-time registration and init-time config publication; prior review feedback fully addressed; thorough tests (holder sharing, copy semantics, responder behavior, deferred-init e2e with bounded load timeout); docs updated in README and ARCHITECTURE.md; fallback values are conservative.
Why not 5: No blocking or meaningful residual issues — the remaining gap is purely stylistic (e.g. setBridgeConfig stores the config reference while getBridgeConfig clones on read; harmless given buildConfiguration produces a stable object and IPC always goes through the copy path).
Architectural flow
sequenceDiagram
participant App
participant Instrument
participant Holder as Process-global holder
participant IPC as ipcMain responder
participant Preload
participant Init as init() / RendererPipeline
App->>Instrument: import instrument (before electron)
Instrument->>IPC: register datadog:bridge-config listener
Instrument->>Holder: seed fallback (mask, [])
Note over App,Preload: Window opens before init()
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: fallback config
IPC-->>Preload: immediate response (no hang)
App->>Init: init(config) after consent gate
Init->>Holder: setBridgeConfig(real config)
Note over App,Preload: New window after init
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: real config
IPC-->>Preload: configured values
Loading
Before:RendererPipeline registered the datadog:bridge-config responder only during init(). When init() was deferred (e.g. user-consent gating), the preload's synchronous sendSync found no listener and blocked the renderer for ~30s.
After: The responder is registered unconditionally at instrument time and reads from a process-global holder seeded with safe fallbacks. init() publishes the real configuration via setBridgeConfig, so windows opened before init load promptly with fallback values and windows opened after init receive the configured privacy level and webview hosts.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
PR Review — Score: 5.0 / 5
This is a well-targeted fix for the deferred-init hang. Registering the datadog:bridge-config responder at instrument time, backed by a process-global holder, directly addresses the root cause while preserving safe fallback behavior. Prior review feedback is fully addressed on the current head: getBridgeConfig() returns a defensive copy, the deferred-init e2e scenario asserts both allowedWebViewHosts and defaultPrivacyLevel before and after init, and README/ARCHITECTURE.md document the lifecycle and pre-init reload caveat. The Symbol.for pattern matches existing instrumentation idempotency, monitor() wraps the IPC callback correctly, and unit plus e2e coverage give strong confidence. I would approve this PR.
Why 5.0: Correct architectural split between instrument-time registration and init-time config publication; all prior findings resolved; thorough tests (holder sharing, copy semantics, responder behavior, bounded pre-init load timeout); documentation complete; fallback values are conservative.
Why not 5: Nothing substantive — ready to merge.
Architectural flow
sequenceDiagram
participant App
participant Instrument
participant Holder as Process-global holder
participant IPC as ipcMain responder
participant Preload
participant Init as init() / RendererPipeline
App->>Instrument: import instrument (before electron)
Instrument->>IPC: register datadog:bridge-config listener
Instrument->>Holder: seed fallback (mask, [])
Note over App,Preload: Window opens before init()
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: fallback config
IPC-->>Preload: immediate response (no hang)
App->>Init: init(config) after consent gate
Init->>Holder: setBridgeConfig(real config)
Note over App,Preload: New window after init
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: real config
IPC-->>Preload: configured values
Loading
Before:RendererPipeline registered the datadog:bridge-config responder only during init(). When init() was deferred (e.g. user-consent gating), the preload's synchronous sendSync found no listener and blocked the renderer for ~30s.
After: The responder is registered unconditionally at instrument time and reads from a process-global holder seeded with safe fallbacks. init() publishes the real configuration via setBridgeConfig, so windows opened before init load promptly with fallback values and windows opened after init receive the configured privacy level and webview hosts.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
PR Review — Score: 5.0 / 5
This is a well-targeted fix for the deferred-init hang. Registering the datadog:bridge-config responder at instrument time, backed by a process-global holder, directly addresses the root cause while preserving safe fallback behavior. The current head (3a3b1cae) includes follow-up commits that land the deferred-init e2e scenario (with bounded pre-init load timeout and before/after assertions on both allowedWebViewHosts and defaultPrivacyLevel) and lifecycle documentation in README and ARCHITECTURE.md. Prior review feedback is fully addressed: getBridgeConfig() returns a defensive copy, the Symbol.for pattern matches existing instrumentation idempotency, monitor() wraps the IPC callback correctly, and unit plus e2e coverage give strong confidence. I would approve this PR.
Why 5.0: Correct architectural split between instrument-time registration and init-time config publication; all prior findings resolved; thorough tests and documentation; fallback values are conservative; no residual correctness or compatibility concerns.
Why not 5: Nothing substantive — ready to merge.
Architectural flow
sequenceDiagram
participant App
participant Instrument
participant Holder as Process-global holder
participant IPC as ipcMain responder
participant Preload
participant Init as init() / RendererPipeline
App->>Instrument: import instrument (before electron)
Instrument->>IPC: register datadog:bridge-config listener
Instrument->>Holder: seed fallback (mask, [])
Note over App,Preload: Window opens before init()
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: fallback config
IPC-->>Preload: immediate response (no hang)
App->>Init: init(config) after consent gate
Init->>Holder: setBridgeConfig(real config)
Note over App,Preload: New window after init
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: real config
IPC-->>Preload: configured values
Loading
Before:RendererPipeline registered the datadog:bridge-config responder only during init(). When init() was deferred (e.g. user-consent gating), the preload's synchronous sendSync found no listener and blocked the renderer for ~30s.
After: The responder is registered unconditionally at instrument time and reads from a process-global holder seeded with safe fallbacks. init() publishes the real configuration via setBridgeConfig, so windows opened before init load promptly with fallback values and windows opened after init receive the configured privacy level and webview hosts.
The reason will be displayed to describe this comment to others. Learn more.
PR Review — Score: 5.0 / 5
This is a well-targeted fix for the deferred-init hang. Registering the datadog:bridge-config responder at instrument time, backed by a process-global holder, directly addresses the root cause while preserving safe fallback behavior. The current head (3a3b1cae) includes the deferred-init e2e scenario (bounded pre-init load timeout; before/after assertions on both allowedWebViewHosts and defaultPrivacyLevel) and lifecycle documentation in README and ARCHITECTURE.md. Prior review feedback is fully addressed: getBridgeConfig() returns a defensive copy, the Symbol.for pattern matches existing instrumentation idempotency, monitor() wraps the IPC callback correctly, and unit plus e2e coverage give strong confidence. I would approve this PR.
Why 5.0: Correct architectural split between instrument-time registration and init-time config publication; all prior findings resolved; thorough tests and documentation; fallback values are conservative; no residual correctness or compatibility concerns.
Why not 5: Nothing substantive — ready to merge.
Architectural flow
sequenceDiagram
participant App
participant Instrument
participant Holder as Process-global holder
participant IPC as ipcMain responder
participant Preload
participant Init as init() / RendererPipeline
App->>Instrument: import instrument (before electron)
Instrument->>IPC: register datadog:bridge-config listener
Instrument->>Holder: seed fallback (mask, [])
Note over App,Preload: Window opens before init()
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: fallback config
IPC-->>Preload: immediate response (no hang)
App->>Init: init(config) after consent gate
Init->>Holder: setBridgeConfig(real config)
Note over App,Preload: New window after init
Preload->>IPC: sendSync(datadog:bridge-config)
IPC->>Holder: getBridgeConfig() (returns copy)
Holder-->>IPC: real config
IPC-->>Preload: configured values
Loading
Before:RendererPipeline registered the datadog:bridge-config responder only during init(). When init() was deferred (e.g. user-consent gating), the preload's synchronous sendSync found no listener and blocked the renderer for ~30s.
After: The responder is registered unconditionally at instrument time and reads from a process-global holder seeded with safe fallbacks. init() publishes the real configuration via setBridgeConfig, so windows opened before init load promptly with fallback values and windows opened after init receive the configured privacy level and webview hosts.
The reason will be displayed to describe this comment to others. Learn more.
Do we want to keep this as a temporary workaround until tracking consent is supported by the SDK, and then remove support for deferred initialization? If so, I'm ok with the PR as it is.
If however, deferred initialization is intended to remain a supported use case independently of tracking consent, we should support it properly by buffering renderer events emitted before the SDK listener is registered.
The reason is that this flow has a high chance of breaking things, an example would be Session Replay, where the initial full snapshot is created on page load and is simply lost if the listener is not registered yet, potentially leading to broken replays. Missing initial RUM events could also cause correlation issues.
Do we want to keep this as a temporary workaround until tracking consent is supported by the SDK, and then remove support for deferred initialization? If so, I'm ok with the PR as it is.
If however, deferred initialization is intended to remain a supported use case independently of tracking consent, we should support it properly by buffering renderer events emitted before the SDK listener is registered.
The reason is that this flow has a high chance of breaking things, an example would be Session Replay, where the initial full snapshot is created on page load and is simply lost if the listener is not registered yet, potentially leading to broken replays. Missing initial RUM events could also cause correlation issues.
Consent tracking is a deferred initialization, as we won't want to send anything before consent is granted but we should have more control to avoid losing data.
Supporting other deferred initialization cases would be more in the realm of early data collection, I don't think it is a current concern. However, deferred initialization should not lead to the SDK breaking the customer app.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
bcaudan
force-pushed
the
bcaudan/config-sync
branch
2 times, most recently
from
cdn34dd
left a comment
Motivation
Importing
@datadog/electron-sdk/instrumentbeforeelectronbut deferringinit()(a common pattern whengating on user consent) made the first window/frame load hang for ~30s. The injected preload does a
synchronous
ipcRenderer.sendSync('datadog:bridge-config')at frame commit, but the responder was onlyregistered by
init(). With no responder, Electron blocked the renderer until the sendSync timeout.Fixes #150 (RUM-17016).
Changes
datadog:bridge-configresponder unconditionally at instrument time, so the preload'ssynchronous request always resolves immediately, whether or not
init()has run.(
maskprivacy, no extra webview hosts). It must be a process global because theinstrumentandinit()entry points ship in separate bundles.init()(viaRendererPipeline) now updates the holder with the real config instead of owning theresponder. A window loaded before
init()reads the fallback; one loaded after reads the real config.ARCHITECTURE.mdand the pre-init reload caveat in the README.Test instructions
require('@datadog/electron-sdk/instrument')beforeelectronand do NOT callinit().BrowserWindowand load any content.sendSync ... without listenerswarning.init(...), open a second window, and confirm it picks up the configuredallowedWebViewHosts/defaultPrivacyLevel.Checklist