✨ Add monitorPromise and migrate .catch(monitorError) call sites - #4976
Draft
BenoitZugmeyer wants to merge 1 commit into
Draft
✨ Add monitorPromise and migrate .catch(monitorError) call sites#4976BenoitZugmeyer wants to merge 1 commit into
BenoitZugmeyer wants to merge 1 commit into
Conversation
Introduce a promise-rejection counterpart to monitor/callMonitored. monitorPromise reports a rejection to telemetry and swallows it (resolves to undefined), mirroring how callMonitored returns undefined on throw. An optional mapError argument transforms the rejection before reporting, covering sites that wrap the error in a descriptive new Error(...). This is an additive, behavior-preserving change: today monitorPromise is just promise.catch(monitorError). It lays the groundwork for the monitor context propagation rework, where a later context-aware version of monitorPromise will capture the active SDK context at call time so async rejections route to the originating SDK's telemetry. Migrate all 19 .catch(monitorError) sites across the SDKs to monitorPromise, including the two that wrapped the error via a custom .catch handler. Allow monitorPromise as a known-safe floating call in no-floating-promises so terminal call sites read naturally without a void prefix.
Bundles Sizes Evolution
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 88130a0 | Docs | View more details | Give us feedback! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Promise rejections from SDK-internal async work (session persistence, fetch pipelines, remote configuration, profiling) are currently routed to telemetry via
.catch(monitorError). This pattern has two gaps that block the upcoming monitor-context-propagation rework:new Error(...)require a custom.catchhandler, fragmenting the pattern.Changes
monitorPromiseto theMonitorinterface in@datadog/js-core/monitor: reports a rejection to telemetry and swallows it (resolves toundefined), mirroringcallMonitored's on-throw return. An optionalmapErrorargument transforms the rejection before reporting, replacing the custom.catchhandlers that wrapped errors in descriptive messages..catch(monitorError)call sites acrossbrowser-core,browser-rum-core,browser-rum, andbrowser-logstomonitorPromise(...), including the two wrapped-error sites.monitorPromiseas a known-safe floating call in the@typescript-eslint/no-floating-promiseslint rule, so terminal call sites read naturally without avoidprefix.monitorPromisethrough@datadog/browser-core.This is an additive, behavior-preserving change: today
monitorPromiseis justpromise.catch(monitorError). It lays the groundwork so the later context-aware version can capture the active SDK context at call time without touching call sites again.Test instructions
yarn dev) and trigger an async code path that rejects (e.g. block the intake endpoint so a fetch rejects, or force a session-store write failure). Confirm the rejection is still reported to telemetry and does not surface as an unhandled rejection in the console.debug: truein the init config), confirm rejected-promise errors still log[MONITOR] ...to the console.Checklist