fix(observability): avoid mutating captured error names#5432
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 21:50:51 UTC
⏸️ Suggested Action - Manual Review Review summary Blockers
Nits — 5 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agent
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5432 +/- ##
==========================================
- Coverage 94.41% 94.37% -0.04%
==========================================
Files 550 550
Lines 44143 44146 +3
Branches 14632 14632
==========================================
- Hits 41677 41663 -14
- Misses 1791 1808 +17
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Motivation
eventNamepreviously mutated the originalErrorinstance'sname, which throws for Error-like objects with read-onlyname(e.g.DOMExceptionfromAbortSignal.timeout) and can crash observability/error handlers.Description
ErrorwheneventNameis provided, setting the original error as the synthetic error'scauseand copying the original stack to preserve triage context.namedCaptureErrorto return the originalErrorunmodified when noeventNameis given, and to construct a newError(err.message, { cause: err })withnameset toeventNamewhen provided.Object.definePropertyto assign thestackon the synthetic error to avoid writable/descriptor issues.captureErrorandcaptureReviewFailurewith aDOMExceptiontimeout and a frozenErrorto ensure the original objects are not mutated and the synthetic error preservesmessage,stack, andcause.Testing
npx vitest run test/unit/selfhost-sentry.test.tsand the tests passed.npm run test:coverage -- test/unit/selfhost-sentry.test.tsand the targeted tests passed, though global coverage thresholds fail when only a single test file is executed in isolation.npm run typecheckand it failed due to unrelated, pre-existing TypeScript issues elsewhere in the repository that are outside the scope of this change.Codex Task