mswjs / interceptors

Low-level network interception library.
https://npm.im/@mswjs/interceptors
MIT License
537 stars 123 forks source link

fix: abort pending requests when the interceptor is disposed #393

Open JesusTheHun opened 1 year ago

JesusTheHun commented 1 year ago

This PR aims to solve issue https://github.com/mswjs/msw/issues/778.

I took the following approach :

Unhandled requests are not affected.

Since this only affects Node.js, I implemented the logic into FetchInterceptor and ClientRequestInterceptor

kettanaito commented 1 year ago

Let's have a discussion on this one, I think I know how we can approach it in an elegant manner. Will gather my thoughts and post here once I have a minute.

JesusTheHun commented 10 months ago

@kettanaito do you think you can find some time in the coming weeks to handle this PR ? I think I can allocate an afternoon next week if you want to discuss this.

howagain commented 10 months ago

Wanted to voice my support for this. As being able to test abort controllers is an important part of unit testing. Hoping this PR solves https://github.com/mswjs/msw/discussions/1646

jd-carroll commented 8 months ago

Copying my 2cents here as well...

I think this change really should be in the RequestHandler not the interceptors.

Check out mswjs/msw#778 for a more full discussion.

JesusTheHun commented 8 months ago

Wanted to voice my support for this. As being able to test abort controllers is an important part of unit testing. Hoping this PR solves https://github.com/mswjs/msw/discussions/1646

@howagain MSW 2.0 includes my first PR that fixes AbortSignal. So if you manually abort the request, you should be able to test that. If not, this PR won't be enough because what this PR does is to abort all requests during MSW shutdown. It gives you no control while the tests are running. You may workaround this if you stop and start the server during the test itself, which introduces new issues, if you run yours tests concurrently for example.

This could be a nice improvement to build on top of this PR. Although, given that this PR has not been merged after 5 months, I will not go further before it has been merged.

Maintainers have a limited bandwidth. I understand it can be frustrating but we are working hundreds of hours for the benefits of strangers on the internet. For free.

Be kind ❤️