test(vitest): cover msw preload regression#9262
Conversation
Overall package sizeSelf size: 6.71 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 437.94 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 2ff3217 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9262 +/- ##
==========================================
- Coverage 96.57% 96.52% -0.05%
==========================================
Files 918 918
Lines 121564 121564
Branches 20803 20684 -119
==========================================
- Hits 117396 117342 -54
- Misses 4168 4222 +54 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-07-10 08:52:07 Comparing candidate commit 2ff3217 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2305 metrics, 53 unstable metrics.
|
There was a problem hiding this comment.
The getErrorStack try/catch fix in telemetry/logs/index.js is correct and directly resolves the Vite/MSW crash. One subtlety worth the author's attention: the change from value.stack (truthy check) to 'stack' in value (property existence) in hasErrorStack silently expands the cause-detection to objects with falsy stack values (null, undefined, '') — those objects are now popped from log args and treated as error causes rather than formatted into the message, which is a behavioral change from before this PR.
📊 Validated against 8 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 1d9ed63 · What is Autotest? · Any feedback? Reach out in #autotest
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d9ed63aec
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 218dc19c50
ℹ️ 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".
|
/datadog autotest review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
More details
The core fix is sound: hasErrorStack now uses types.isNativeError (which returns true for Error subclasses on Node.js ≥22 without reading .stack) and an own-data-property descriptor check for plain objects, and getErrorStack wraps .stack reads in try/catch. The 74 tests all pass across the changed files. One intentional behavioral narrowing: objects whose stack property is an inherited property rather than an own data property are no longer treated as error causes (old code: maybeError.stack truthy check; new code: own descriptor check). This is a deliberate, low-risk tradeoff to avoid ever triggering getter-based stack formatters during cause detection.
📊 Validated against 12 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit aa247d3 · What is Autotest? · Any feedback? Reach out in #autotest
What does this PR do?
Adds a Vitest 3.2.6 + MSW 2.14.6 integration regression test for dd-trace's supported Vitest preload path:
NODE_OPTIONS="-r dd-trace/ci/init --import dd-trace/register.js"The test verifies that importing MSW and performing an MSW-backed fetch does not fail while dd-trace telemetry/logging handles loader diagnostics.
Motivation
The original #9260 failure path was:
httphelpers, so loading MSW also loadsgraphql.__module_export__transforms for those GraphQL files.Failed to find injection points for: ["__module_export__"].log.error(e).e.stack.//# sourceMappingURL=....${inlineMap}text as base64 JSON, which producedSyntaxError: Unexpected token '�'.After merging current
master, the MSW/Vitest repro passes even with this PR's production log/telemetry guard changes removed. The relevant master fix is #9212, which avoids leaving the problematic rawsourceMappingURLmarker in the rewriter source for vite-node to misparse.This PR keeps the focused regression coverage so the supported Vitest preload path stays protected.
Additional Notes
Validation:
masterinto this branch.SPEC=vitest.msw npm run test:integration:vitestpasses.