MetaMask / MetaMask/metamask-extension

Research and resolve errors in reconfiguring fetch in use-snow.js

Open
#24,759 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

INVALID-ISSUE-TEMPLATE
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

https://github.com/MetaMask/metamask-extension/pull/24631 fixes a problem on MV3 builds, whereby attempting `chrome.notifications.create` resulted in failure because that code attempts to use a scuttled fetch.

We attempted to fix the problem by setting a custom tamed `fetch` on the global to be scuttled. This required that we cache the fetch on globalThis. However, once that was attempted (see the PRs branch state as of b31e0aff79f369fb3569da4e2af1f15ce42bfd7e), a strange problem occured:

> Okay, so there is a new issue that I don't understand.
>I am now caching fetch by placing this at the top of use-snow.js:
> ```
> const { fetch, Error, Object } = globalThis;
> const { hasOwn, defineProperty } = Object;
> ````
> That fetch is something that is defined within sentry-install.js
> The problem is that the fetch called within > https://github.com/chromium/chromium/blob/main/extensions/renderer/resources/image_util.js#L23-L25 is now that cached fetch (defined within sentry-install.js) and not the tamedFetch that was added to the realm with defineProperty

We could observe that the problem did not occur if we reached into `globalThis.fetch` and got the original/native fetch, as was done in 3a4a55b780ef8a0bc355ad0daeb91521b0eb863b. However, that was just for demonstration purposes.

We do not yet understand why the tamed fetch was not called when `globalThis.fetch` was cached, nor why it was called when only the native fetch nested within the sentry fetch was cached.

For the time being, to unblock the release of MV3, we have added an exception to the scuttling of fetch. However, we want to investigate this further and remove that exception with a solution that gets to the root of the problem.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with use-snow.js and sentry-install.js, then compare the behavior described in PR 24631 and commit b31e0aff79f369fb3569da4e2af1f15ce42bfd7e with the native-fetch experiment in 3a4a55b780ef8a0bc355ad0daeb91521b0eb863b. Reproduce the MV3 chrome.notifications.create failure and trace the fetch used by Chromium's image_util.js. Done means the fetch behavior is explained and the temporary fetch-scuttling exception can be removed safely.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.