Fix broken failure telemetry: restore Clipper.logger assignment#657
Merged
Conversation
4f02a93 to
ab581b7
Compare
Two V2 module-level loggers (Log.ErrorUtils.sendFailureLogRequest and userDataBoundaryHelper) reach for the static `Clipper.logger` slot, which V2 never set up. V1's clipper.tsx contained: Clipper.logger = new CommunicatorLoggerPure(...) That setter fired when the V1 Mithril sidebar bootstrapped via injection. V2 replaced the architecture (worker opens renderer window directly, no sidebar injection) and clipper.tsx stopped executing in V2's flow -- so the V1 setter never fired again, and no V2-equivalent was established. The bug has been latent throughout V2 development. Commit 8f0dad5 later deleted the dead V1 source file, but the runtime behavior was already broken before that cleanup. Normal Aria telemetry (InvokeClipper, ClipToOneNoteAction, HandleSignInEvent, GetNotebooks, all context properties) is unaffected -- those flow through per-worker AriaLoggerDecorator instances that never touch Clipper.logger. The static-slot path is only used by module-level functions that have no `this` context to hold a logger on. Affected failure-scenario events that were silently dropped: - UnhandledExceptionThrown (extensionBase self.onerror handler -- SW safety net for any uncaught crash) - UnclippablePage (webExtensionWorker scripting.executeScript probe failure on chrome:// / about:// / file:// URLs) - GetChangeLog (changelog HTTP fetch failure after version update) - JsonParse (userDataBoundaryHelper EUDB endpoint malformed JSON during sign-in) Each call into Clipper.logger.logFailure threw `TypeError: Cannot read properties of undefined (reading 'logFailure')` inside the error handler. The browser silently swallows exceptions from self.onerror (no recursive re-fire), so neither the original error nor the telemetry-mechanism error reached observability. Fix: assign Clipper.logger = this.logger in the ExtensionBase constructor immediately after WorkerPassthroughLogger is created. Routes module-level failure logs through the same per-worker chain that normal telemetry uses. Verified via SW DevTools repro: after invoking the clipper once to spawn a worker, throwing an unhandled exception in the SW console now POSTs a WebClipper.Failure.UnhandledExceptionThrown event to browser.pipe.aria.microsoft.com/Collector/3.0/. The SW-boot window (failures before any worker exists, so WorkerPassthroughLogger has zero fan-out targets) remains a pre-existing architectural gap shared with V1 -- out of scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ab581b7 to
6904bed
Compare
aanchalbhansali
approved these changes
May 12, 2026
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.
Summary
Wires up
Clipper.loggerinExtensionBase— a static slot that two V2 module-level loggers depend on, but that V2 never set up. This is not a "normal telemetry is broken" PR: per-worker Aria logging for all user-initiated flows has been working fine. Only specific failure-scenario events were being silently dropped.What WAS working (unaffected by this bug)
Per-worker Aria logging via
LogManager.createExtLogger()→AriaLoggerDecorator. All the normal observability:InvokeClipper,ClipToOneNoteAction,HandleSignInEvent,GetNotebooksThese flow
worker → Ariadirectly; they never touchClipper.logger.What was NOT working
Module-level functions that need a logger reference but have no
thiscontext (no instance to hold a logger on) reach for the static slotClipper.logger. Affected call sites:Log.ErrorUtils.sendFailureLogRequest— invoked from:extensionBase.ts:316→GetChangeLog(changelog fetch failure after version update)extensionBase.ts:380→UnhandledExceptionThrown(SWself.onerror— the safety net for any uncaught crash)webExtensionWorker.ts:151→UnclippablePage(scripting.executeScript probe failure)userDataBoundaryHelper.ts:69→JsonParse(sign-in EUDB endpoint returned malformed JSON)Each of these called
Clipper.logger.logFailure(...)onundefined, throwingTypeError: Cannot read properties of undefined (reading 'logFailure')inside the error handler itself. The browser silently swallows exceptions thrown byself.onerror(no recursive re-fire), so the failure was invisible — neither the original error nor the telemetry-mechanism error reached observability.Most impactful loss:
UnhandledExceptionThrownis the SW safety net for any unanticipated crash. Production telemetry has been blind to novel SW exceptions throughout V2.Root cause
V1's
src/scripts/clipperUI/clipper.tsxcontained:That setter fired when the V1 Mithril sidebar bootstrapped via injection. V2 replaced the whole architecture — worker opens renderer window directly, no sidebar injection — and
clipper.tsxstopped executing in the V2 flow. The V1 setter was never re-established in V2 and never fired again. The bug has been latent throughout V2 development. Commit 8f0dad5 later deleted the dead V1 source file, but the runtime behavior was already broken before that cleanup.Why it stayed invisible until now:
TypeErrorthrown insideself.onerroris silently swallowed by the browserFix
One line in
ExtensionBaseconstructor, immediately afterWorkerPassthroughLoggeris created:Routes module-level failure logs through the same per-worker
AriaLoggerDecoratorchain that normal telemetry uses.Test plan
npm run build:prod→ zero errors, tslint passesgrep "Clipper.logger\s*="finds 1 match in the shipped JS)setTimeout(() => { throw new Error("repro"); }, 0);in SW console →WebClipper.Failure.UnhandledExceptionThrownPOST tobrowser.pipe.aria.microsoft.com/Collector/3.0/confirmed with full Aria event payload (FailureType=Unexpected, Id=ExtensionBase, full stack)Out of scope
The SW-boot window (errors fired before any per-tab worker exists →
WorkerPassthroughLoggerhas zero fan-out targets) is a pre-existing architectural gap shared with V1 — V1'sClipper.loggerwas set inside the sidebar at injection time, so SW errors before sidebar load were also lost in V1. Filed as follow-up; full coverage would require an SW-levelAriaLoggerDecoratorindependent of workers.