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

@MartinKolarik MartinKolarik commented Aug 25, 2021

  • 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.

gr2m
gr2m approved these changes Aug 25, 2021
Copy link
Member

@gr2m gr2m left a comment

Thanks Martin, great PR 💐

Loading

@gr2m
Copy link
Member

@gr2m gr2m commented Aug 25, 2021

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

Loading

@gr2m gr2m enabled auto-merge (squash) Aug 25, 2021
@gr2m gr2m merged commit ac8e33d into nock:main Aug 25, 2021
17 checks passed
Loading
@github-actions
Copy link

@github-actions github-actions bot commented Aug 25, 2021

🎉 This PR is included in version 13.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Loading

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

Successfully merging this pull request may close these issues.

None yet

2 participants