msw: Using delay('infinite') leaves unresolved promises which make jest warn

I just ran into this issue as well. It’d be great to have support for this as currently quite a lot of my tests have to use an arbitrary delay which makes them flaky.

Not sure what changed, but ‘infinite’ now leaves Jest hanging forever… 🤦‍♂️

@kettanaito I’ve submitted a PR into mswjs/interceptors to fix this issue. Would you mind taking a look ?

I think with the new interceptors architecture we should be able to hard-cancel all pending promises on process.on('exit') and friends. It’s a matter of force-resolving/rejecting this Promise queue. At least, this is a good place to start, as all interceptors facilitate AsyncEventEmitter to await potential listeners.

Contributions are welcome!

Hey, @russomp. Thanks for showing interest in working on this! Let’s go through the points you’ve mentioned.

the problem I saw with using the request/response hooks is that we get the InteractiveRequest which I’m not sure how to abort, since what we actually need is the physical ClientRequest and/or XMLHttpRequest to call .destroy() and .abort() on respectively as far as I can tell.

That’s correct, and the Interceptors used to expose a reference to the request instance but they don’t do that anymore. Anyhow, MSW shouldn’t handle this directly, but instead, the Interceptors must expose an API to do that implicitly do that for us, as the references on that level are still available and it’s the Interceptors that should have an API to abort any pending requests.

assuming the above is correct, was wondering what you thought about emitting a new request-internal event that gives us the actual “internal” request instances

I think this inverses the control if I understand your suggestion correctly. If you mean this:

consumer -> emit "request-internal" -> get request refs -> call ref.abort()

Then the control flow here is incorrect. We already have consumer (MSW) / library (Interceptors) communication in a form of interceptors API. So, I think no changes are needed on MSW’s side to try to solve this. The Interceptors library needs to implicitly kill any pending requests of the interceptor’s kind upon interceptor.dispose() of the respective interceptor.

Your signal based-take is interesting but I’m not sure to what extent we can reuse that across different interceptors (XHR/fetch). The way I would recommend approaching this implementation is to stay request module-agnostic, so what we come up with works for any interceptor the very same way. That’s why using the Interceptor class as the common ground for this sounds like a good initial idea:

1. The interceptor has access to an emitter that manages request/response events.
2. The interceptor can create a “request” listener and store any performed request internally.
3. Upon dispose(), the interceptor goes through each stored request and performs request.abort() (or other, based on the intercepted request type).

What remains as a question is whether this behavior can be safely applied to all interceptors. Whether you’d always want to abort pending requests when disposing of an interceptor. In other words, to think about the cases when you wouldn’t want that to happen. I can’t think of any at the top of my head right now.

This relation would work because the following call stack occurs at the end of a test run:

-> test runner: afterAll()
  -> MSW: server.close()
    -> Interceptor(s): [i].dispose()

hi @kettanaito I ran into this at the work the other day and would love to take this on if no one else is already working on it. I’m pretty new to the msw libraries but was doing some digging yesterday based on the above discussion and wanted to follow-up with some thoughts.

the problem I saw with using the request/response hooks is that we get the InteractiveRequest which I’m not sure how to abort, since what we actually need is the physical ClientRequest and/or XMLHttpRequest to call .destroy() and .abort() on respectively as far as I can tell. (let me know if I am missing something here though since like I said this is a bit new to me 😊)

assuming the above is correct, was wondering what you thought about emitting a new request-internal event that gives us the actual “internal” request instances we need for this with their associated ids to consume or providing an internal setter to register new internal requests with the interceptor. Alternatively, I was messing around with overriding the signal property in NodeClientRequest like so

// src/interceptors/ClientRequest/NodeClientRequest.ts
...
constructor(
    [url, requestOptions, callback]: NormalizedClientRequestArgs,
    options: NodeClientOptions  // extended to take abort controller from interceptor
  ) {
    const requestOptionsWithSignalOverride = {
      ...requestOptions,
      signal: options.requestController.signal,
    }

    // if user tries to abort using passed signal setup listener to call signal override to abort
    function onAbort() {
      options.requestController.abort()
      requestOptions.signal?.removeEventListener('abort', onAbort)
    }
    requestOptions.signal?.addEventListener('abort', onAbort)

    super(requestOptionsWithSignalOverride, callback)
    ...

that would then allow the interceptor to call requestController.abort() when dispose is called while preserving any user passed signals but I wasn’t sure if that would work as nicely for XMLHttpRequest (although I did see some examples about how we might make that work). The abort controller approach is also not as extenisble though if we want to do anything else with the pending requests being tracked with the first approach in the future I suppose.

@icatalina, yeah, killing pending requests on server.close() may be one way to do it. The challenge here is to have an isomorphic way to abort pending requests in Node.js. I think that’s natively supported by XMLHttpRequest and ClientRequest, which covers most of the cases in Node.js. It’s not supported in fetch, speaking of the global fetch support, but we may be able to circumvent it by creating an internal AbortController. My only concern here is how this is supposed to work when the consumer passes a custom AbortController.

I would love to see someone come up with a prototype for this. This seems to be a feature added to @mswjs/interceptors library. We don’t currently keep any track of what requests are performed but it looks like we need to do that now in order to do requests.map(r => r.abort()) on the global request abort thing. Keeping track of requests on MSW’s side is also possible but we still need some means from the interceptors library to perform the request cancellation. Because of that, it makes sense to move the whole logic to the lowest common ground, being the interceptors library.

Here’s an approximate way we can approach this.

We have a Interceptors.prototype.dispose method that’s called when disposing of each interceptors (read “server.close()”)"

https://github.com/mswjs/interceptors/blob/cac847c322a80667d6fcc2a3b0318257d74abb15/src/Interceptor.ts#L169

We can extend this method in each particular interceptor and implement request cancellation.

// src/interceptors/ClientRequest/index.ts
class ClientRequestInterceptor extends Interceptor {
  private pendingRequests: Map<string, Request>

  protected setup() {
    // ...the existing setup here

    // Keep all dispatched requests internally.
    this.emitter.on('request', (request, requestId) => {
      this.pendingRequests.set(requestId, request)
    })

    // Potentially remove requests from the pending requests map.
    // The problem here is that "response" is not the only way
    // a request may settle. It may also error. Maybe hooking into
    // the "response" event is unnecessary, and we should instead
    // check the request state in "this.abortPendingRequest" before
    // aborting the request. 
    this.emitter.on('response', (response, request, requestId) => {
      this.pendingRequests.delete(requestId)
    })
  }

  public dispose() {
    super.dispose()
    this.abortPendingRequests()
  }

  private abortPendingRequests() {
    // Go through every request that's still pending
    // and abort it. 
    for (const [requestId, request] of this.pendingRequests) {
      request.abort()
    }
  }
}

—Source

Moving the request cancellation logic to each individual interceptor makes sense as the base Interceptor class is rather generic and can implement any interceptors (e.g. WebSocket events interception that has no “abort” by definition).

@finalight, do you want to try:

rest.all('*', () => new Promise(() => undefined))

I’m not sure if I’m dreaming, but it seems to be working fine 😅