MetaMask / MetaMask/metamask-extension

setupSentry.js's integrateLogging() crashes Sentry.init() against getter-only @sentry/core logger

Open Beginner friendly
#45,206 1 comment 0 reactions 0 assignees View on GitHub
INVALID-ISSUE-TEMPLATE Sev1-high size-S ta-ai-fixable ta-triaged team-extension-platform
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

**File:** `app/scripts/lib/setupSentry.js` (`integrateLogging()`)

---

## Problem

`integrateLogging()` monkey-patches `@sentry/core`'s shared `logger.log`/`logger.error` via direct assignment (`sentrySdkLogger.log = ...`, `sentrySdkLogger.error = ...`) so SDK-internal debug logs route through the app's own logger. The installed `@sentry/core` version now exposes `logger.log`/`logger.error` as getter-only properties, so the direct reassignment throws `TypeError: Cannot set property error of # which has only a getter`.

This function runs at the very top of `bootstrap.ts`'s Sentry setup, before anything else in the background/UI boot sequence, and only when `METAMASK_DEBUG` is true — which `development/build/set-environment-variables.js` hardcodes to `true` for any non-production build (`isDevBuild`), unconditionally. The crash is synchronous and uncaught, so `Sentry.init()` never completes and the whole extension fails to boot ("Background connection unresponsive").

**Why this hasn't been hit in normal development:** it only fires when Sentry is actually compiled into the build (`--sentry` webpack flag) AND `METAMASK_DEBUG` is true, which isn't the default combination for a plain local dev/test build — most local development doesn't pass `--sentry` at all.

Found while validating the sentry-remote-flag PR cluster (`#43228`/`#44538`/`#44540`/`#44541`) locally with `yarn env:e2e webpack --test --sentry`.

---

## Solution

Guard each reassignment (e.g. wrap in `try/catch`, or check `Object.getOwnPropertyDescriptor(sentrySdkLogger, loggerType)?.set` before assigning) so an SDK version exposing these as getter-only doesn't crash `Sentry.init()`. Losing the redirected debug logging in that case is an acceptable degradation — it has no bearing on sampling, init, or transport behavior.

---

## Acceptance Criteria

- [ ] `integrateLogging()` does not throw when `@sentry/core`'s `logger.log`/`logger.error` are getter-only.
- [ ] A build with `METAMASK_DEBUG=true` and `--sentry` boots successfully regardless of the installed `@sentry/core` version's property descriptors.

---

## Labels

`team-extension-platform`, `area-sentry`

Contributor guide

Open the contributing guide

Research direction

Start in app/scripts/lib/setupSentry.js at integrateLogging(), then review the Sentry setup entry point in bootstrap.ts. Reproduce the failure with yarn env:e2e webpack --test --sentry and inspect the logger property descriptors. Done means getter-only logger.log and logger.error no longer abort Sentry.init(), and the Sentry build boots successfully with METAMASK_DEBUG=true.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript, webpack
Domain
observability-sre
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.