possible mistaken commit in v20.x branch webstreams

[Waldenesque]

Version

version 20.13.0 and higher

Platform

all

Subsystem

webstreams

What steps will reproduce the bug?

No response

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

No response

What do you see instead?

I was curious about the freshness of the webstreams code in v20 versus main, so I looked at the commit history of the following folder:

https://github.com/nodejs/node/tree/main/lib/internal/webstreams

I noticed something odd about the following commits:

4d3923a
8e82451
66556f5

It appears there was an initial fix attempt that had some problem, then a revert, and finally a correct fix. These three commits seem identical in main branch and v22.x branch, while v21.x branch includes three equivalent commits in the same chronological order on slightly different dates. However v20.x branch looks different.

https://github.com/nodejs/node/tree/v20.x/lib/internal/webstreams

It appears the broken initial fix attempt never made it into v20.x branch in the first place. The first relevant commit seems to be the final correct fix. However a little over a month later, the older revert commit seems to have been applied over top of the correct fix. I'm guessing this was an accidental mistake?

If so, it looks like the mistaken revert modified a couple of test files, and also incorrectly removed a couple of "port.unref();" lines from the "transfer.js" file. If the commit notes listed in CHANGELOG_V20.md are correct, this presumably broken transfer.js file shipped in Node version 20.13.0, and is still broken in the current v20.14.0-proposal branch scheduled to ship this coming Tuesday 2024-05-28.

https://github.com/nodejs/node/tree/v20.14.0-proposal/lib/internal/webstreams

I don't know anything about the specifics of any of this stuff, so hopefully I'm not wasting someone's time by writing this. However it looks fishy enough to me that I thought it was worth reporting.

Thanks for your time.

Additional information

No response

[RedYetiDev]

@tsctx
@mcollina
@jasnell

(Original commit authors)

[climba03003]

The same revert PR is being deployed in two different version.
f2dfe0fa80 which is 20.12.0
8d20b641a2 which is 20.13.0

The proper fix d0a6f35 is deployed in 20.12.0

I think it is accidentally included the Revert in 20.13.0 since 20.12.0 is correctly excluded it.

[marco-ippolito]

I guess we can just revert "the revert commit" 8d20b641a2 that shouldnt have landed?
#53144

[marco-ippolito]

Fixed in v20.14.0