🐛 Drain SDK idle-callback queue before flushing in e2e tests#4569
🐛 Drain SDK idle-callback queue before flushing in e2e tests#4569thomas-lebeau wants to merge 2 commits intomainfrom
Conversation
The SDK defers resource event emission onto a `requestIdleCallback`-backed task queue (with a 1s timeout). The previous 200ms in-page setTimeout in `waitForRequests` could return before that idle callback fired, leaving events in the queue when `pagehide` triggered `sendBeacon` — so they were silently dropped from the captured intake registry. Replace the fixed wait with two `requestIdleCallback` round-trips (with a 500ms watchdog for pages where rIC is throttled or unavailable, e.g. the empty /flush page used during teardown). This eliminates the residual flake exposed by tests firing two back-to-back `page.click` calls (e.g. microfrontend.scenario.ts fetch/xhr/feature-operation tests), without requiring per-test workarounds. Verified with 50× repeats of the full microfrontend suite (1050/1050 pass).
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: fa10f7c | Docs | Datadog PR Page | Give us feedback! |
| let done = false | ||
| const finish = () => { | ||
| if (!done) { | ||
| done = true | ||
| resolve() | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion: this is unnecessary, just use resolve instead of finish: it doesn't matter if resolve is called twice
Promise resolve() is idempotent, so the manual `done` flag and `finish` wrapper are unnecessary — call resolve() directly from both the watchdog timeout and the idle-callback chain.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa10f7ccd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // first batch of tasks are also processed. A 500ms watchdog covers pages where | ||
| // requestIdleCallback is throttled or unavailable (e.g. the empty /flush page during | ||
| // teardown). | ||
| setTimeout(resolve, 500) |
There was a problem hiding this comment.
When requestIdleCallback is throttled, this watchdog can resolve flushEvents() before the SDK's own task queue is forced to run: packages/core/src/tools/taskQueue.ts schedules SDK work with IDLE_CALLBACK_TIMEOUT = ONE_SECOND, but this fallback proceeds after 500 ms. In that throttled/no-idle context the helper can still call waitForServersIdle() while resource-emission tasks remain queued, so the flake this path is meant to cover can still reproduce; the fallback needs to wait at least as long as the SDK timeout or force the same queue to run.
Useful? React with 👍 / 👎.
3 participants
Motivation
E2E tests (notably the microfrontend fetch/XHR scenarios) were flaky because the SDK defers some bookkeeping — particularly resource event emission — onto an idle-callback task queue. The previous
waitForRequestshelper just slept 200ms viasetTimeout, which was not always enough for those deferred tasks to run before the test asserted on captured events.Changes
setTimeoutinwaitForRequestswith tworequestIdleCallbackround-trips to drain the SDK's idle-callback queue (including tasks enqueued by the first batch).requestIdleCallbackis throttled or unavailable (e.g. the empty/flushpage during teardown).Test instructions
yarn test:e2eChecklist