Identify the root cause of the "Premature close" error

[danjm]

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: https://github.com/mafintosh/pump/issues/25

• there are two separate "premature close" errors. One pre-existed this PR (and v12.0.0) https://github.com/MetaMask/metamask-extension/pull/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" https://github.com/MetaMask/metamask-extension/pull/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. In #26336 we stopped logging them, as the are clogging sentry.

We should identify and resolve the root cause, and then remove code in the error handlers that ignores these errors.

[Aliceonly]

hey there, my metamask always meets Premature close since it switch to a custom network. detail:https://github.com/MetaMask/metamask-extension/issues/26326 it leads my metamask can't use,cuz it always crush when open it, and lost my Secret Recovery Phrase. give any help plz, it's a fatal error. there's much important data in the extension @danjm

[legobeat]

others have experienced similar problems: https://github.com/mafintosh/pump/issues/25

Just noting that from my understanding pump/end-of-stream are only really compatible with legacy/v2 streams and using them with newer versions may cause undefined behavior like this due to related API changes.

AFAICT we're not utilizing those packages in runtime but only for browserify's legacy streams, though.

[legobeat]

• 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" https://github.com/MetaMask/metamask-extension/pull/2070

Might be something there? Looks like that reasoning behind that line-change resolving a leak might have applied to v2 streams (which was used at the time, with pump) but not for the v3 streams currently used?

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

[legobeat]

not stale?