MetaMask / MetaMask/metamask-extension

[Bug]: Legacy extension streams not re established after service worker error recovery

Open Beginner friendly
#45,124 1 comment 0 reactions 0 assignees View on GitHub
cs-triaged external-contributor needs-triage regression-RC-13.44.0 Sev1 team-extension-platform type-bug
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

### Describe the bug

In `provider-stream.ts`, the error-recovery path inside `onDisconnectDestroyStreams` calls only `setupExtensionStreams()` but omits `setupLegacyExtensionStreams()`. Every other call site in the codebase always calls both functions together, so this inconsistency leaves legacy provider streams permanently broken after a service worker restart caused by an error.

There are two concrete failure modes depending on timing:

**Mode A EXTENSION_MESSAGES.READY arrives BEFORE the 1000 ms timeout:**
`onMessageSetUpExtensionStreams` runs first and correctly calls both setup functions, setting `extensionStream` to a non-null value. When the `setTimeout` fires 1 second later, `setupExtensionStreams()` is called a second time. This creates a brand-new `extensionPort` without disconnecting the previous one, overwrites the module-level `extensionMux / extensionEip1193Channel / extensionCaipChannel` variables, and registers an additional `onDisconnect` listener on the new port — leaving the old port permanently orphaned (memory leak + duplicate pipeline).

**Mode B EXTENSION_MESSAGES.READY arrives AFTER the 1000 ms timeout:**
The `setTimeout` fires first and sets `extensionStream` to a non-null value. When `EXTENSION_MESSAGES.READY` arrives, `onMessageSetUpExtensionStreams` checks `if (!extensionStream)` which is now `false`, so it skips both setup calls entirely. `setupLegacyExtensionStreams()` is never called legacy dApp connections are silently broken for the rest of the session.

### Expected behavior

After a service worker error disconnect and reconnect, both `setupExtensionStreams()` and `setupLegacyExtensionStreams()` should be called together in the error-recovery path, consistent with all other call sites (`initStreams` and `onMessageSetUpExtensionStreams`).

### Screenshots/Recordings

_No response_

### Steps to reproduce

This is a code-level race condition triggered during service worker error recovery. The scenario described in the existing TODO comment (https://github.com/MetaMask/metamask-extension/issues/31893) running two or more dApps simultaneously and triggering a service worker reset is the most reliable way to hit it.

1. Open two dApps simultaneously, both connected to MetaMask.
2. Trigger a service worker reset (e.g. navigate away and back on one tab, or disable/re-enable the extension).
3. Observe the port disconnect error: "Could not establish connection. Receiving end does not exist."
4. Watch the 1-second recovery timer fire via `setTimeout(setupExtensionStreams, 1000)`.
5. Legacy provider channel (`METAMASK_EIP_1193_PROVIDER` via the legacy mux) is no longer functional for the rest of the session.

### Error messages or log output

```shell
// Console warning emitted during error recovery (provider-stream.ts:343):
Error: Could not establish connection. Receiving end does not exist. Resetting the streams.

// After recovery — legacy provider traffic silently dropped, no error thrown.
// In Mode A — orphaned port, duplicate pipeline:
MaxListenersExceededWarning: Possible EventEmitter memory leak detected
```

### Where was this bug found?

Internal release testing

### Version

13.44.0

### Build type

None

### Browser

Chrome

### Operating system

Linux

### Hardware wallet

_No response_

### Additional context

Root cause — three call sites, only one is wrong:

// ✅ initStreams() — line 357-358
setupExtensionStreams();
setupLegacyExtensionStreams();

// ✅ onMessageSetUpExtensionStreams() — line 295-296
setupExtensionStreams();
setupLegacyExtensionStreams();

// ❌ onDisconnectDestroyStreams() line 344 BUG HERE
setTimeout(setupExtensionStreams, 1000);
// setupLegacyExtensionStreams() is missing

Proposed fix (one line):

// Before:
setTimeout(setupExtensionStreams, 1000);

// After:
setTimeout(() => {
setupExtensionStreams();
setupLegacyExtensionStreams();
}, 1000);

Note: this also resolves the Mode A duplicate-setup race because after `setupExtensionStreams()` and `setupLegacyExtensionStreams()` are called atomically in the same tick, `onMessageSetUpExtensionStreams` will see `extensionStream !== null` and correctly skip re-initialization.

Related: #31893 (the TODO comment referencing the race condition is already in this exact function)

Found via static analysis of the cloned repository at commit: 9def44e (main, 2026-08-01).

### Severity

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in provider-stream.ts at onDisconnectDestroyStreams and compare its recovery path with initStreams and onMessageSetUpExtensionStreams. Verify that recovery initializes both extension and legacy streams after the timer, and confirm that the existing stream behavior no longer permits duplicate setup or leaves legacy provider traffic unavailable.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api
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.