fix(error-tracking): wait for debug ID injection before sourcemap upload - #487
Conversation
|
relevant comment from the previous PR: #470 (comment) |
There was a problem hiding this comment.
Pull request overview
Friend, this PR synchronizes esbuild artifact rewriting with Error Tracking sourcemap uploads.
Changes:
- Adds an artifact-readiness barrier to shared context.
- Delays and deduplicates sourcemap uploads until injection finishes.
- Adds barrier and debug-ID regression coverage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/core/src/types.ts |
Defines artifact barrier APIs. |
packages/factory/src/helpers/context.ts |
Implements barrier lifecycle. |
packages/factory/src/helpers/context.test.ts |
Tests resolution, rejection, and reuse. |
packages/plugins/injection/src/index.ts |
Marks esbuild artifacts pending. |
packages/plugins/injection/src/esbuild.ts |
Resolves the barrier after rewriting. |
packages/plugins/error-tracking/src/index.ts |
Gates and deduplicates uploads. |
packages/plugins/error-tracking/src/index.test.ts |
Adds synchronization and debug-ID tests. |
packages/tests/src/_jest/helpers/mocks.ts |
Extends context mocks. |
Suppressed comments (1)
packages/plugins/error-tracking/src/index.test.ts:111
- Split the mapped promise and the push into named steps. The current nested calls violate the repository’s call-argument convention and make this assertion setup harder to follow.
debugIdsAtUpload.push(
...(await Promise.all(
javascriptOutputs.map(({ filepath }) => extractDebugId(filepath)),
)),
);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| const handleOrQueueSourcemaps = async () => { | ||
| if (context.artifactsPending) { | ||
| context.queue(handleSourcemaps()); |
There was a problem hiding this comment.
Addressed in commit 299ffff0. The upload promise is now assigned to the named sourcemapsPromise local before it is passed to context.queue().
| ); | ||
| const plugin = getPlugins(arg)[0]; | ||
|
|
||
| const buildReportHook = plugin.buildReport!(getMockBuildReport()); |
There was a problem hiding this comment.
Addressed in commit 299ffff0. The test now creates a named buildReport before invoking the hook. I also addressed the additional occurrence by awaiting Promise.all into a named debugIds local before passing the values to debugIdsAtUpload.push(). Focused tests pass 7/7, along with ESLint and Error Tracking typechecking.
buranmert
left a comment
buranmert
left a comment
There was a problem hiding this comment.
that's a bit beyond my understanding of JS build plugins for me to propose a better way to achieve that.
i'm fine as long as our end-to-end tests pass 👍
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #487...
Detected 1 merge blocker(s) to address: 🔴 Blocking |
299ffff to
40d9fe7
Compare
c9384d1
into
DataDog:master
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
What
Add an explicit artifacts-ready barrier so Error Tracking waits until esbuild finishes rewriting JavaScript and source-map artifacts before discovering sourcemaps and extracting debug IDs.
The early esbuild build-report callback queues the gated upload instead of blocking the later injection callback. Concurrent lifecycle attempts share one upload promise.
Why
RUM-18038 live testing showed esbuild uploaded all sourcemaps but extracted debug IDs for 0/N because upload ran before debug-ID injection completed.
How it works
.jsand.mapartifact is ready.ddDebugId, and uploads that value with the final JavaScript and sourcemap.The uploader does not inject or modify the bundle. The synchronization prevents it from reading or uploading artifacts while esbuild is still rewriting them.
Tests
Jira: https://datadoghq.atlassian.net/browse/RUM-18038