[jgindin]

Description

This is the 4th of a stack of 4 PRs to improve error handling.

• Add method to send composer errors to the usage tracking service
• Replace usages of console.log, console.warn, etc... with usages of the ErrorLogger service

Pre-launch Checklist

• I signed the [CLA].
• I read the [Contributors Guide].
• I read the [Style Guide].
• I have added updates to the [CHANGELOG].
• I updated/added relevant documentation.
• My code changes (if any) have tests.
• If my branch is on fork, I have verified that scripts/e2e_test.sh passes.

[github-actions]

⚡ A2UI Composer PR Preview

Your automated preview is successfully live (commit fd294e7):
👉 Launch PR Preview

Note: This environment will be wiped automatically when the PR is merged or closed.

[gemini-code-assist]

Code Review

This pull request introduces a centralized diagnostic logging and error-handling framework across the A2UI composer workspace, including a new ErrorLogger service, real-time schema validation in the Monaco editor, and debounced error overlays in the Angular, React, and Lit adapters. The review feedback highlights several opportunities for improvement: deduplicating consecutive identical marker sets in the Monaco editor to prevent log flooding, cleaning up duplicate mock properties in tests, using the correct warning-level logging method in raw-frame.ts, escaping a dot in a regular expression in errors.ts, and preserving the error class name alongside the message in the ErrorLogger service.

[suyangw-g]

Review Summary

Scoped note: #197 stacks on #196, so its diff against main contains the whole 4-PR series. This review covers the incremental delta (git diff pr-196..pr-197 — 51 files, +1414/−309). My 27 comments on #196 still apply to the shared base and aren't repeated here.

The console.* → ErrorLogger migration is thorough and mechanically clean — ~30 call sites converted, withTag() applied consistently, and the accompanying spec updates are diligent. Extracting handleConsoleLog() in HostCommunication removes real duplication, the new errors.spec.ts and ga4-usage-tracking.service.spec.ts suites are well built, and the monaco-editor.spec.ts marker rewrite (real IMarker shapes via a factory instead of loose casts) is a solid improvement. It's also good to see e2e coverage added for the error surfaces I flagged as missing on #196.

I'm requesting changes because the headline feature doesn't work end-to-end, and CI is still red.

🔴 Blockers

1. CI is still failing — the watchdog fix is half-applied. This PR correctly switched errorLogger.error → errorLogger.warn for the unresponsive branch, but neither branch sets level, and the IFRAME_UNRESPONSIVE_ERROR: prefix was never added. Both raw-frame.spec.ts watchdog tests still fail. → raw-frame.ts:243, raw-frame.ts:248

2. error_category is never populated, so the telemetry dimension is dead on arrival. errors.ts:92 is the only production caller and omits it, so GA4 receives event_label: 'UNKNOWN_ERROR' and error_category: 'UNKNOWN_ERROR' for 100 % of composer_error events. The specs pass only because they supply it by hand. → errors.ts:92

3. The dedupe signature silently drops most error telemetry. ${sourceTag}:${line}:${column}:${invalidProp} — all three coordinates are undefined for any error that isn't a Monaco marker (storage, startup, share, bridge…), so they all collapse to [Shell]:undefined:undefined:undefined and only the first one per 5-second window is ever reported. The signature also excludes message. → errors.ts:87

4. The Errors badge now counts info and log messages. The trigger moved from CONSOLE_LOG envelopes to errorLogger.errorStream$ with no level filter, while this same PR converts ~15 console.log calls into logger.info. The badge will be non-zero on every cold start from routine startup chatter. → composer-workspace.ts:85

5. A new e2e test asserts nothing. The test named "captures cross-frame preview errors, renders non-crashing 350ms debounced UI error overlay, and assigns [Previewer] log provenance" has zero assertions and ends with // Ignored to avoid brittleness. It can never fail. → json-error-handling.e2e.ts:181

⚠️ Privacy concern worth a decision before merge

ga4-usage-tracking.service.ts:316 sends params.message verbatim to Google Analytics. Composer error messages routinely embed user content — JSON fragments from the user's own layout, file paths, API error bodies. GA4's terms prohibit PII. I'd send category + coordinates only, or truncate hard and strip quoted literals.

🟠 Significant, non-blocking

• Telemetry is emitted from a UI component. Errors is a dockview panel — if it's never mounted, no composer_error events are sent at all. This belongs in ErrorLogger or a root-provided subscriber.
• Type error in a spec: ComposerErrorTelemetryParams.message is required, but ga4-usage-tracking.service.spec.ts:500 and :525 omit it. Vitest doesn't typecheck, so it passes at runtime while tsc/lint would reject it.
• Test-only scaffolding in production code: window.a2uiCatalogManagement = this plus a public handshakeHistoryIndex signal whose JSDoc says "Used by integration tests". It extends an existing a2uiHostCommunication precedent, but that precedent is worth containing rather than growing.
• waitForTimeout is reintroduced in the very PR that removes it elsewhere — json-error-handling.e2e.ts:89 (1000 ms in beforeEach) and renderer-integration-and-telemetry.e2e.ts:33 (150 ms inside waitForPreviewSettled).
• Third copy of the same pattern: "hydrate from getHistoryBuffer() + dedupe via WeakSet" now lives in events.ts, data-model.ts, and partially catalog-management.ts.
• data-model.ts:114 deep-compares with JSON.stringify, which is key-order sensitive — the repo already has stableStringify.
• errors.ts:90 — the 5 s setTimeout is never cleared on destroy.
• errors.ts:82 — /instance\.([^ ]+) /i requires a trailing space, so a message ending in the property name never matches.

📋 Documentation

No Fixes #… link, and the CHANGELOG / documentation boxes are unchecked despite adding a new abstract method (trackComposerError) to the exported UsageTrackingService — a breaking change for any external subclass — plus a new exported interface and a breaking CatalogSchemaResolver constructor signature. The e2e checkbox is also unchecked while the PR adds a 253-line e2e suite and rewrites three others.

---

Blockers 2 and 3 are the ones I'd most want resolved: as written, the telemetry this PR exists to add would produce a single undifferentiated UNKNOWN_ERROR bucket with most events missing. ErrorLogItem.sourceTag already carries [Monaco] / [ChatParser] / [Previewer] / [Shell] / [Storage], so a small mapping function would give you a genuinely useful dimension.

[suyangw-g]

Two blockers on the new code since my last pass, both in host-communication.ts. Everything else from my original review verified as addressed — the reporter extraction, the PII removal, the timer elimination and the e2e de-flaking all landed well.