Skip to content

Use late-bound noop function when disabling console#10110

Merged
Gudahtt merged 1 commit intodevelopfrom
use-late-bound-noop-when-disabling-console
Dec 19, 2020
Merged

Use late-bound noop function when disabling console#10110
Gudahtt merged 1 commit intodevelopfrom
use-late-bound-noop-when-disabling-console

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Dec 19, 2020

The disable-console script introduced in #10040 used an arrow-function no-op function to replace console.log and console.info. This replacement function was early-bound to the this context of the disable-console script, because that's how arrow functions work.

This violates an assumption baked into Sentry, which also replaces the console functions. It wraps them in a function it uses to track console logs as breadcrumbs. This wrapper function blows up for some reason if the "original" console function is early-bound to a this value of undefined.

This resulted in various UI freezes. One example is during onboarding, when using Firefox with Enhanced Tracking Protection set in "strict" mode. After submitting a password in the 'Create wallet' flow, the Sentry console wrapper would throw and leave the user stuck on the loading screen.

By replacing the no-op arrow function with a no-op function declaration, the problem has been resolved.

Relates to #10097

@Gudahtt Gudahtt requested a review from a team as a code owner December 19, 2020 05:28
@Gudahtt Gudahtt requested a review from danjm December 19, 2020 05:28
The `disable-console` script introduced in #10040 used an arrow-
function no-op function to replace `console.log` and `console.info`.
This replacement function was early-bound to the `this` context of the
`disable-console` script, because that's how arrow functions work.

This violates an assumption baked into Sentry, which also replaces the
`console` functions. It wraps them in a function it uses to track
console logs as breadcrumbs. This wrapper function blows up for some
reason if the "original" `console` function is early-bound to a `this`
value of `undefined`.

This resulted in various UI freezes. One example is during onboarding,
when using Firefox with Enhanced Tracking Protection set in "strict"
mode. After submitting a password in the 'Create wallet' flow, the
Sentry `console` wrapper would throw and leave the user stuck on the
loading screen.

By replacing the no-op arrow function with a no-op function
declaration, the problem has been resolved.

Relates to #10097
@Gudahtt Gudahtt force-pushed the use-late-bound-noop-when-disabling-console branch from 392c8f4 to 0e676b3 Compare December 19, 2020 05:28
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0e676b3]
Page Load Metrics (508 ± 44 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33554374
domContentLoaded2955975079244
load2965995089244
domInteractive2945975079244

@Gudahtt Gudahtt merged commit 889ca62 into develop Dec 19, 2020
@Gudahtt Gudahtt deleted the use-late-bound-noop-when-disabling-console branch December 19, 2020 20:28
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants