fix: ignore error when getTokenStandardAndDetails fails

[sahar-fehri]

Description

Fixes Sentry issue when we are not able to determine token contract standard.

Related issues

Fixes: #17287

Manual testing steps

(I was not able to repro with my NFTs but to simulate, we can go to the core fct getTokenStandardAndDetails and add
throw new Error('Unable to determine contract standard'); at the start of the fct;

1. Go NFT page
2. Click on any NFT
3. Click Send
4. You should be able to send the NFT

Screenshots/Recordings

Before

Screen.Recording.2024-10-23.at.02.24.34.mov

After

Screen.Recording.2024-10-23.at.02.44.38.mov

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]

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]

Builds ready [ddb4ae2]

• 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, 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 (1901 ± 79 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1695	2393	1910	168	81
		domContentLoaded	1685	2321	1877	153	73
		load	1696	2391	1901	165	79
		domInteractive	17	182	56	33	16
		backgroundConnect	8	68	22	18	8
		firstReactRender	48	176	99	33	16
		getState	4	59	19	20	10
		initialActions	0	1	0	0	0
		loadScripts	1207	1890	1402	150	72
		setupStore	10	72	27	24	11
		uiStartup	1894	2569	2124	193	93

Bundle size diffs [🚀 Bundle size reduced!]

• background: 0 Bytes (0.00%)
• ui: 0 Bytes (0.00%)
• common: -7 Bytes (-0.00%)

[sahar-fehri]

This change wont affect native transfers;
It also wont affect ERC20 transfers because for ERC20 transfers tokenCanBeTreatedAsAnERC20 here should be true making details defined and skipping the call to getTokenStandardAndDetails.
This should only affect NFT assets;

This is throwing because the code detects that there is a missing property which is symbol in the case of the nfts i have tested with (here) and because the providedDetails has standard property it will go into the else and call getTokenStandardAndDetails. (still not sure why getTokenStandardAndDetails would throw for so many contracts, it might be good to add the contract address and tokenId in the error log)

The code is detecting that the symbol is missing because the nftDetails symbol is inside providedDetails.collection,
but this piece of code
is expecting the symbol inside the providedDetails object;

The properties that it is checking for an NFT are basically "name, symbol, tokenId" i dnt think tokenId can be undefined for an NFT.
Looking at the number of users affected by this; i would expect that most of them had missingProperty= symbol

Adding sth similar to this;

let missingProperty = STANDARD_TO_REQUIRED_PROPERTIES[ providedDetails.standard ]?.find((property) => { if (providedDetails.collection && property === 'symbol') { return providedDetails.collection[property] === undefined; } return providedDetails[property] === undefined; });
Would make the code go into this if statement
and not make the call to getTokenStandardAndDetails.

However, the symbol inside collection is not always defined either, there will be cases where our third party provider did not return it.

In the case; the code will go into else and call getTokenStandardAndDetails;
For that i have also removed the "throw error"; I don't see why it is necessary to throw when this call fails; I have also tested this and my transactions are going through with the call to getTokenStandardAndDetails failing

[sahar-fehri]

There are other places in codebase that call getTokenStandardAndDetails; all of them are either within a try/catch or called using useAsyncResult which should also catch the error

[metamaskbot]

Builds ready [c84f198]

• 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, 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 (1957 ± 222 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1600	3375	1961	447	215
		domContentLoaded	1557	3355	1910	416	200
		load	1595	3368	1957	462	222
		domInteractive	25	181	55	34	16
		backgroundConnect	6	303	49	81	39
		firstReactRender	44	176	93	38	18
		getState	5	142	24	36	18
		initialActions	0	1	0	0	0
		loadScripts	1108	2626	1400	332	159
		setupStore	10	134	31	36	17
		uiStartup	1738	3953	2214	635	305

Bundle size diffs [🚨 Warning! Bundle size has increased!]

• background: 0 Bytes (0.00%)
• ui: 0 Bytes (0.00%)
• common: 48 Bytes (0.00%)

[salimtb]

can you add a unit test for this ?

[gambinish]

I wonder if we should still log the error that gets caught to make future troubleshooting easier? Do we have a logger setup for this kind of thing?

Otherwise lgtm 👍

[metamaskbot]

Builds ready [eb405c4]

• 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, 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 (2739 ± 584 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1599	5201	2751	1225	588
		domContentLoaded	1583	5133	2710	1210	581
		load	1597	5152	2739	1215	584
		domInteractive	27	245	83	64	31
		backgroundConnect	9	73	33	19	9
		firstReactRender	46	355	148	100	48
		getState	5	95	26	24	11
		initialActions	0	1	0	0	0
		loadScripts	1146	3879	1989	892	429
		setupStore	11	92	40	24	11
		uiStartup	1761	5651	3077	1400	672

Bundle size diffs [🚨 Warning! Bundle size has increased!]

• background: 0 Bytes (0.00%)
• ui: 0 Bytes (0.00%)
• common: 48 Bytes (0.00%)