-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix(premature close): gracefully end extension stream muxes to prevent "Premature close" errors #37400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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. |
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [d5f5e65]
UI Startup Metrics (1269 ± 99 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
d5f5e65 to
64e70d8
Compare
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [64e70d8]
UI Startup Metrics (1280 ± 71 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
64e70d8 to
4b6c51f
Compare
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [4b6c51f]
UI Startup Metrics (1156 ± 76 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
…ation to prevent pipeline errors
4b6c51f to
e72ec5e
Compare
Builds ready [e72ec5e]
UI Startup Metrics (1257 ± 78 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
LGTM !! |
Description
This PR is an AI-generated fix aiming to fix the #1 error in Sentry (3.8M occurrences/month) by adding graceful shutdown handlers to extension context streams, preventing "ERR_STREAM_PREMATURE_CLOSE" errors.
The error was initially discussed in this Slack thread.
Disclaimer
Since we haven't found a reliable way to manually reproduce these "Premature close" errors, there's no "before/after proof" that this PR fixes it.
Problem
When underlying transport streams (e.g.,
WindowPostMessageStream,ExtensionPortStream) close or end abruptly—during page navigation, tab closure, or extension disconnection—theObjectMultiplexinstances in extension context do not gracefully shut down. This results in "Premature close" errors being thrown by the stream pipeline.Solution
Added graceful shutdown handlers to extension context streams that:
once('close')andonce('end')listeners to underlying transport streams!destroyed && !writableEnded) and callmux.end()before pipeline error detection kicks inKey Design Decision: Page Context vs Extension Context
Why handlers are ONLY in extension context streams:
inpage.jsprovider-stream.tsphishing-stream.tscookie-handler-stream.tsThis approach mirrors the solution in #33470 for CAIP streams.
Changelog
CHANGELOG entry: Fixed "Premature close" stream errors in extension context by adding graceful shutdown handlers
Related issues
Fixes: #26337
Fixes: #35241
Fixes: #21078
Manual testing steps
(disclaimer: the error is hard to reproduce, even with these manual testing steps)
chrome://extensions→ MetaMask → "service worker"chrome-extension://{extension-id}/home.html#link?u=/buyand verify it redirects cleanly without errorsExpected: Clean shutdowns with no "Premature close" errors in console
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Add graceful shutdown handlers to extension-context multiplexers for provider, phishing, and cookie-handler streams, plus tests; clarify no handlers in page-context in inpage.
close/endto avoidERR_STREAM_PREMATURE_CLOSEin:app/scripts/streams/provider-stream.ts(pageMux,extensionMux)app/scripts/streams/phishing-stream.ts(phishingPageMux,phishingExtMux)app/scripts/streams/cookie-handler-stream.ts(cookieHandlerPageMux,cookieHandlerMux)logStreamDisconnectWarningand sendingMETAMASK_STREAM_FAILUREwhere applicable.app/scripts/inpage.js: Add documentation explaining why graceful shutdown handlers are not added in page context.provider-stream.filter.test.tsphishing-stream.filter.test.tscookie-handler-stream.filter.test.tsWritten by Cursor Bugbot for commit e72ec5e. This will update automatically on new commits. Configure here.