fix: `gasFeeEstimates` property undefined #29312

vinistevam · 2024-12-18T09:30:23Z

Description

This PR fixes an issue created by Sentry where the property gasFeeEstimates is undefined.
Added an early return when the property is undefined and covered the scenario with unit tests.

Related issues

Fixes: #27501 #27241

Manual testing steps

Go to this test dapp
Block requests against gas-api
Start a send or contract interaction
Verify the console

Screenshots/Recordings

block-gas-api.webm

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2024-12-18T09:30:37Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2024-12-18T10:03:03Z

Builds ready [0c8d002]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1712 ± 172 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	308	3134	1575	553	265
		domContentLoaded	1414	3069	1677	347	167
		load	1423	3140	1712	358	172
		domInteractive	25	104	47	27	13
		backgroundConnect	7	105	39	28	13
		firstReactRender	16	98	42	30	14
		getState	4	87	17	20	10
		initialActions	0	1	0	0	0
		loadScripts	985	2215	1245	260	125
		setupStore	6	84	23	24	12
		uiStartup	1599	3715	2071	464	223

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 13 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2024-12-18T10:42:21Z

Builds ready [2e0d45d]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1661 ± 35 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1514	1865	1660	78	38
		domContentLoaded	1493	1795	1632	75	36
		load	1505	1802	1661	73	35
		domInteractive	15	78	35	16	7
		backgroundConnect	9	61	23	15	7
		firstReactRender	16	98	46	32	15
		getState	4	68	22	24	12
		initialActions	0	77	4	17	8
		loadScripts	1084	1373	1202	71	34
		setupStore	6	56	12	13	6
		uiStartup	1704	2392	2012	245	118

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 13 Bytes (0.00%)
common: 0 Bytes (0.00%)

matthewwalsh0 · 2024-12-18T16:02:36Z

ui/pages/confirmations/hooks/useTransactionFunctions.js

+      if (!gasFeeEstimates) {
+        return;
+      }
+
      if (!gasFeeEstimates[gasFeeEstimateToUse]) {


Could we achieve the same with optional chaining here?

True, pushed here.

metamaskbot · 2024-12-19T07:58:17Z

Builds ready [18f8b8e]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1664 ± 66 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1458	1989	1663	138	66
		domContentLoaded	1445	1935	1632	138	66
		load	1464	1986	1664	137	66
		domInteractive	24	93	46	22	10
		backgroundConnect	10	64	26	16	7
		firstReactRender	16	100	44	32	15
		getState	5	63	14	14	7
		initialActions	0	1	0	0	0
		loadScripts	1059	1483	1228	114	55
		setupStore	6	80	16	19	9
		uiStartup	1664	2356	1972	226	109

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 9 Bytes (0.00%)
common: 0 Bytes (0.00%)

Applied changes suggested

fix: gasFeeEstimates property undefined

0c8d002

vinistevam added the team-confirmations Push issues to confirmations team label Dec 18, 2024

Merge branch 'main' into fix/sentry-gas-fee-estimates-undefined

2e0d45d

vinistevam marked this pull request as ready for review December 18, 2024 13:44

vinistevam requested a review from a team as a code owner December 18, 2024 13:44

matthewwalsh0 previously requested changes Dec 18, 2024

View reviewed changes

fix: use optional chaining

18f8b8e

vinistevam mentioned this pull request Dec 19, 2024

[Bug]: Gas - TypeError: Cannot set properties of undefined (setting 'gasFeeEstimates') #27241

Closed

jpuri approved these changes Dec 19, 2024

View reviewed changes

vinistevam requested a review from matthewwalsh0 December 20, 2024 08:28

OGPoyraz approved these changes Dec 20, 2024

View reviewed changes

vinistevam added this pull request to the merge queue Dec 20, 2024

cryptotavares approved these changes Dec 20, 2024

View reviewed changes

Merged via the queue into main with commit 3fd27a9 Dec 20, 2024
80 checks passed

vinistevam deleted the fix/sentry-gas-fee-estimates-undefined branch December 20, 2024 11:47

github-actions bot locked and limited conversation to collaborators Dec 20, 2024

metamaskbot added the release-12.11.0 Issue or pull request that will be included in release 12.11.0 label Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: `gasFeeEstimates` property undefined #29312

fix: `gasFeeEstimates` property undefined #29312

vinistevam commented Dec 18, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024

metamaskbot commented Dec 18, 2024

metamaskbot commented Dec 18, 2024

matthewwalsh0 Dec 18, 2024

vinistevam Dec 19, 2024

metamaskbot commented Dec 19, 2024

fix: gasFeeEstimates property undefined #29312

fix: gasFeeEstimates property undefined #29312

Conversation

vinistevam commented Dec 18, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Dec 18, 2024

metamaskbot commented Dec 18, 2024

metamaskbot commented Dec 18, 2024

matthewwalsh0 Dec 18, 2024

Choose a reason for hiding this comment

vinistevam Dec 19, 2024

Choose a reason for hiding this comment

metamaskbot commented Dec 19, 2024

fix: `gasFeeEstimates` property undefined #29312

fix: `gasFeeEstimates` property undefined #29312

vinistevam commented Dec 18, 2024 •

edited

Loading