Skip to content

API Review: Diagnostic Monitor API#5565

Open
chetanpandey1266 wants to merge 12 commits into
user/chetanpandey/DiagnosticMonitorAPIfrom
user/chetanpandey/DiagnosticMonitorAPI-draft
Open

API Review: Diagnostic Monitor API#5565
chetanpandey1266 wants to merge 12 commits into
user/chetanpandey/DiagnosticMonitorAPIfrom
user/chetanpandey/DiagnosticMonitorAPI-draft

Conversation

@chetanpandey1266

Copy link
Copy Markdown
Contributor

Add Diagnostic Monitor API spec — Introduces ICoreWebView2DiagnosticMonitor, a new observation-only API that
delivers diagnostic signals (e.g., network failures) from all WebViews, profiles, and the environment through a
single DiagnosticReceived event. Host apps create a monitor via CreateDiagnosticMonitor and opt in per category
using AddDiagnosticReceivedFilter.

@chetanpandey1266 chetanpandey1266 added the API Proposal Review WebView2 API Proposal for review. label Apr 15, 2026

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prose, Grammar, and Voice Review

Line-by-line pass focused on grammar, tone, and narrative voice consistency. Suggested corrections are inline below. The overall prose quality is good — these are polish items to bring the spec to Microsoft documentation standard.

Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Sample Issue: The destructor calls m_monitor.Reset() without first calling remove_DiagnosticReceived. This teaches an anti-pattern — callbacks may fire during or after destruction. Best-practice samples should demonstrate proper cleanup order.

Suggested correction:

DiagnosticComponent::~DiagnosticComponent()
{
    if (m_monitor)
    {
        // Unsubscribe before releasing the monitor to
        // ensure no callbacks arrive during teardown.
        m_monitor->remove_DiagnosticReceived(
            m_diagnosticToken);
        m_monitor.Reset();
    }
}

Comment thread specs/DiagnosticMonitor.md

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

Comment thread specs/DiagnosticMonitor.md Outdated

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

Comment thread specs/DiagnosticMonitor.md Outdated

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

Comment thread specs/DiagnosticMonitor.md Outdated

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

Comment thread specs/DiagnosticMonitor.md Outdated

@shrinaths shrinaths left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline

Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md
Chetan Pandey and others added 2 commits April 27, 2026 14:16
…onvention alignment

- Fix grammar: 'An empty JSON' -> 'An empty JSON object' (2x)
- Clarify ambiguity: 'mixed-content blocks' -> 'mixed-content blocked requests'
- Use prescriptive language: 'should match' -> 'must match' in API contract
- Add missing 'that' in relative clause + imperative voice for CoTaskMemFree
- Remove markdown bold (**any**) from IDL comments (plain text only)
- Add missing comma after introductory phrase 'On failure'
- Split run-on sentence in event handler comment
- Add missing comma before 'but' in compound sentence
- Remove 'NetworkContext' reference from profile scope comment
- C++ destructor: unsubscribe before Reset() for proper cleanup order
- C# Dispose: unsubscribe before Dispose() for proper cleanup order
- Add _environment field declaration in filtered C# sample
- Name opaque error codes: ERR_NAME_NOT_RESOLVED (-105), ERR_TIMED_OUT (-7)
- Align section headers: 'COM' -> 'Win32 C++', '.NET C#' -> '.NET, WinRT'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@chetanpandey1266 chetanpandey1266 force-pushed the user/chetanpandey/DiagnosticMonitorAPI-draft branch from c4ce409 to 0894cc0 Compare April 27, 2026 09:28
@david-risney david-risney self-requested a review May 6, 2026 18:00
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md
Comment thread specs/DiagnosticMonitor.md Outdated
[propget] HRESULT Timestamp(
[out, retval] INT64* value);

/// Returns category-specific diagnostic data as a JSON

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer giving data an API versus representing it with JSON or some other format inside a string. Strings are a weak contract that skips our API tooling for compat validation and documentation, isn't discoverable via intellisense, gives the end devs more work to actually get a property value out.

If this was only for telemetry and the details string varied dramatically with no set form - like devtools console.log messages - then it would make sense to have it as a string. But in this case so far there's one kind of networking event that has a well-defined shape for a bunch of different kinds of errors.

Can we add a Details object that varies type based on the kind of error? Eg CoreWebView2DiagnosticReceivedEventArgs.Details property that for networking errors is a CoreWebView2DiagnosticNetworkErrorDetails class. Or if the networking error only corresponds to an existing event - use that existing event as the details object. And remove the SetDiagnosticFilter and instead allow the end dev to implement their own filtering using the details object.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated description to reflect intent of the API. The real intent of this api is for host apps to collect additional information in rare, hard to debug, tricky conditions and relay the information back to us. In that sense it is like a console logging api...

Comment thread specs/DiagnosticMonitor.md Outdated
Comment thread specs/DiagnosticMonitor.md Outdated
@chetanpandey1266 chetanpandey1266 force-pushed the user/chetanpandey/DiagnosticMonitorAPI-draft branch from daaab37 to 82a6439 Compare May 14, 2026 10:04
Chetan Pandey and others added 5 commits June 12, 2026 12:59
- Rename COREWEBVIEW2_DIAGNOSTIC_SCOPE_WEB_VIEW to ..._WEBVIEW to match
  existing one-word convention (e.g. COREWEBVIEW2_*).
- Rename JSON field "protocol" to "scheme" and update prose to "URI scheme".
- Strengthen errorCode documentation: mark net_error_list.h as the
  authoritative source, note stability and additive evolution, and
  instruct consumers to treat unknown codes as generic failures.
- Convert GetDetailsAsJson() to a DetailsAsJson property (COM [propget]
  and WinRT { get; }); update C++ and .NET samples accordingly.
- Drop the "returns {} for unrecognized categories" sentence; only one
  populated category exists.
- Add HRESULT Close() to ICoreWebView2DiagnosticMonitor, mirroring the
  ICoreWebView2SharedBuffer pattern for deterministic teardown.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Timestamp, drop profileName

- Rename COREWEBVIEW2_DIAGNOSTIC_CATEGORY_NETWORK_ERROR to
  ..._NETWORK_REQUEST (WinRT: NetworkError -> NetworkRequest). The
  category emits a per-request lifecycle signal, not failures only;
  errorCode == 0 means success.
- Rewrite the category description to spell out exactly which requests
  fire it (navigation, sub-resource, fetch/XHR, dedicated/shared
  workers, attached service workers) and what is excluded (other host
  apps sharing the user data folder, CSP violation reports).
- Switch Timestamp to wall-clock: COM double seconds since UNIX epoch,
  WinRT Windows.Foundation.DateTime. Enables correlation with other
  timestamped telemetry sources.
- Drop profileName from the filter schema, both code samples, and
  intro prose to keep the filter consistent with the emitted Details.
- Update intro and example prose to refer to "network requests" rather
  than "network errors".

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The filter JSON schema and the DetailsAsJson schema vary per category,
so they belong on each COREWEBVIEW2_DIAGNOSTIC_CATEGORY value rather
than on the category-agnostic methods. As we add more categories the
method-level docs would otherwise grow into a per-category dumping
ground.

- Move NETWORK_REQUEST filter schema (errorCode/statusCode/uriPattern/
  httpMethod, wildcard rules, AND/OR semantics) and Details schema
  (errorCode/statusCode/httpMethod/elapsedTime/scheme/uri, with field
  descriptions) onto the COREWEBVIEW2_DIAGNOSTIC_CATEGORY_NETWORK_REQUEST
  enum value. Update the top-of-enum doc comment to point readers there.
- Strip SetDiagnosticFilter down to category-agnostic behavior
  (empty filter = match all, replace-on-recall, E_INVALIDARG on bad
  JSON or schema mismatch) and link to the enum for the schema.
- Strip DetailsAsJson down to category-agnostic behavior and link to
  the enum for the schema.
- Mirror the same restructure on the WinRT enum and DetailsAsJson
  property.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rewrite the Background and Description sections to position the
Diagnostic Monitor API as an on-demand field diagnostics surface
for non-functioning deployments, not as a general error-handling
or telemetry mechanism. Clarifies that the API is complementary
and tangential to existing error-handling APIs and is intended to
be turned on externally to capture detailed diagnostic data from
a specific misbehaving instance.

Also justifies the JSON-in / JSON-out shape as the right contract
for shipping new categories and fields to deployments already in
the field, and notes that monitors stay inert until SetDiagnosticFilter
is called.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rewrite the Description in third-person declarative voice matching
the convention used in other WebView2Feedback specs (CookieManagement,
ClearBrowsingData, CustomDownload, Autofill). Replace "you/we" and
phrases like "Its purpose is", "matters for", "shape is intentional"
with named subjects ("The monitor", "The host app", "A monitor")
and direct statements. Preserves all content and approximate length.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread specs/DiagnosticMonitor.md Outdated
error paths. When that happens, the only recourse is detailed
logging information from the affected WebView2 instance, often
behind a security boundary that prevents the host app itself
from collecting it inline.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"security boundary prevents host app itself from collecting it inline" needs to be rephrased.

At the end of the day, its the host app that collects and uploads logs and telemetry.

I suppose what you are trying to say is, at present there is no way the host app can collect additional logging information from webview2 that highlights events that lead to a failure

Comment thread specs/DiagnosticMonitor.md Outdated

None of today's WebView2 APIs cover this scenario. Existing
error-handling APIs such as `ServerCertificateErrorDetected` are
interactive, per-instance, and shaped around in-flow decisions —

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reword "in-flow" decisions

Comment thread specs/DiagnosticMonitor.md Outdated
None of today's WebView2 APIs cover this scenario. Existing
error-handling APIs such as `ServerCertificateErrorDetected` are
interactive, per-instance, and shaped around in-flow decisions —
not around capturing rich, free-form diagnostic data from a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"free-form diagnostic data" => capturing rich, varying diagnostic information that can be logged.

Comment thread specs/DiagnosticMonitor.md Outdated
misbehaving deployment after the fact.

The Diagnostic Monitor API addresses this gap. It is an
**observation-only** surface that lets a host app opt in,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"observation-only" => logging

Chetan Pandey and others added 3 commits June 15, 2026 14:08
- Rephrase 'security boundary ... collecting it inline' to clarify
  that today there is no built-in way for the host app to collect
  this additional WebView2 logging data that is not available
  otherwise.
- Reword 'in-flow decisions' to 'real-time decisions'.
- Replace 'free-form diagnostic data' with 'varying diagnostic
  information that can be logged'.
- Replace 'observation-only' with 'logging' (both occurrences) and
  update 'A monitor observes' to 'A monitor captures' for
  consistency with the new logging framing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- COM IDL: change Timestamp from double seconds to INT64 milliseconds
  since the UNIX epoch. Aligns with the INT64 timestamp variable
  already used in the Win32 C++ sample.
- WinRT: change Timestamp from Windows.Foundation.DateTime to Int64
  so the projection mirrors the COM type and matches the long
  timestamp variable already used in the .NET sample.
- Update the doc comment to say 'milliseconds since the UNIX epoch'
  instead of 'seconds since the UNIX epoch'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Proposal Review WebView2 API Proposal for review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants