Skip to content

Michael/fix 2863#2867

Merged
mikicho merged 2 commits into
betafrom
Michael/fix-2863
May 29, 2025
Merged

Michael/fix 2863#2867
mikicho merged 2 commits into
betafrom
Michael/fix-2863

Conversation

@mikicho

@mikicho mikicho commented May 24, 2025

Copy link
Copy Markdown
Member

Resolved #2863 in the beta branch.
I'm wondering if we should backport it to v14, because it's a somewhat breaking change (but probably as desired). WDYT?

@mikicho mikicho requested a review from gr2m May 24, 2025 14:40
@mikicho mikicho changed the base branch from main to beta May 24, 2025 20:26

@gr2m gr2m left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you wether to backport or not, but from my experience it will cause friction for v14 that you don't expect. When in doubt, I'd encourage folks to upgrade instead.

@mikicho mikicho merged commit 0aa44d8 into beta May 29, 2025
11 checks passed
@mikicho mikicho deleted the Michael/fix-2863 branch May 29, 2025 22:06
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 15.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@BridgeAR

Copy link
Copy Markdown
Contributor

It would be much appreciated if this would be backported to 14. It is actually a bad experience if we update to a behavior and then have to revert that in the next major again to the former (more intuitive) one.

@mikicho

mikicho commented Sep 11, 2025

Copy link
Copy Markdown
Member Author

@BridgeAR I'm concerned that this might introduce a breaking change, which could lead to user complaints about the different behavior (even if it's a bug). Nevertheless, I'm ok to attempt a backport and monitor for any issues. Feel free to open a PR.

@BridgeAR

Copy link
Copy Markdown
Contributor

@mikicho I just looked at it for a bit but there are some conflicts in places that I can't determine easily what is right or wrong and I can't find the code change that removed this between 13 and 14. Could you open the backport yourself? I believe that is much easier for you knowing the code base :)

I guess it could be tested by us and others who were facing the issues with the delay to see if the backport would cause any other issues before actually landing the PR.

@mikicho

mikicho commented Sep 12, 2025

Copy link
Copy Markdown
Member Author

@BridgeAR Thanks for taking a look into it. I am currently focusing on different things in Nock.

mikicho pushed a commit that referenced this pull request Apr 30, 2026
This fixes a regression where `delayBody(N)` no longer makes a body look
slow to the caller. Read timeouts on the response stream are expected to
fire when N exceeds them, but in v14 the @mswjs/interceptors-based
pipeline drains the response body before delivering the response event.
Pausing the body source (the v13 mechanism) ended up delaying the
*whole* response — response, data, and end events all moved to
`+delayBodyInMs`, indistinguishable from a slow connection.

Two coordinated pieces fix it:

1. `lib/playback_interceptor.js`: drop the pause/resume mechanism and
   gate only the end-of-response push (`response.push(null)`) on
   `delayBodyInMs`. Schedule that gate inside `respond()` so
   `delay({ head, body })` continues to compound — head defers the
   response event, body then defers the end signal by an additional
   `body` ms. Reply callbacks like `replyWithFile` hand back an
   explicitly-paused `fs.createReadStream`, so call
   `bodyAsStream.resume()` to ensure the body flows.
2. `tests/got/test_delay.js`: the two tests that asserted timing on the
   first `data` event are updated to assert on the `end` event,
   matching the new (and intended) semantics.

Refs: #2867
Fixes: #2863
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delayBody doesn't timeout anymore

3 participants