Skip to content
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

fix(interceptor): don't hang, don't leak resources #2224

Merged
merged 2 commits into from Aug 25, 2021
Merged

Conversation

MartinKolarik
Copy link
Contributor

  • The first commit fixes an issue that caused the last request to hang when using a combination of .times() and .replyWithFile() - the counter was being decremented too early, which meant the if on the next line didn't pass and the last request didn't get a fresh body
  • The second commit fixes a resource leak - because a stream is created here when calling .replyWithFile(), it should be consumed on the first call, and a new one should only be created here on the 2nd call.

The added test catches both of these issues.

Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Thanks Martin, great PR 💐

@gr2m
Copy link
Member

gr2m commented Aug 25, 2021

I just pushed the test and lib updates separately to show that the tests failed before the fix.

@gr2m gr2m enabled auto-merge (squash) August 25, 2021 04:58
@gr2m gr2m merged commit ac8e33d into nock:main Aug 25, 2021
@github-actions
Copy link

🎉 This PR is included in version 13.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants