Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Stop logging pipeline stream errors in the service worker if the…
…y match 'Premature close' (#26336) ## **Description** We have long seen many errors in Sentry that have the message "Premature close". This PR addresses those that occur when the UI is closed, and when a connected dapp is closed. This error is thrown from within the readable-stream module when multiple streams are passed to the same pipeline. We do not fully understand the issue yet, but it is suspected that we are incorrectly triggering or handling the destroy or end of a stream. Some relevant findings: - others have experienced similar problems: mafintosh/pump#25 - there are two separate "premature close" errors. One pre-existed this PR (and v12.0.0) #24533, and one was introduced with that PR - the one that was introduced with that PR goes away if the readable-stream dependency of object-multiplex is revert back to v2.3.3 - "so I think the problem is that we are explicitly calling destroy on some of the stream's in the pipeline, before the pipeline is finished doing whatever it needs to do when when the streams are ended/closed/finished" - if we use `connectionStream.pipe(mux).pipe(connectionStream);` instead of `pipeline(connectionStream, mux, connectionStream`, the error goes away, but that was changed almost 7 years ago in a PR titled "Memory leak fixes - stream and filter life cycles" #2070 - With reference to `setupMultiplex`: - if I change that line to pipeline(connectionStream, mux, (err) => {, the error goes away - but if I change it to pipeline(mux, connectionStream, (err) => {, the error does not go away We understand enough at this point to know that these error are not affecting user experience. For the moment, we are going to explicitly stop logging them, as the are clogging sentry, and [we can revisit the search for root cause in the future ](#26337) Note that there is at least one outstanding case of a "Premature close" error that can occur when the application is loading. This PR does not attempt to remove that error from being logged. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26336?quickstart=1) ## **Related issues** Fixes: #26284 ## **Manual testing steps** 1. Open the UI 2. Clear the background console 3. Close the UI 4. There should be no "Premature close" error ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). 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.
- Loading branch information