mswjs / interceptors

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

Await all "response" listeners to finish #569

Closed mdesousa closed 4 days ago

mdesousa commented 2 months ago

hi, we have code that looks like this:

const interceptor = new BatchInterceptor(...); // console.log to print requests and responses
const p1 = fetch('x.com');
const p2 = fetch('y.com');
await Promise.all([p1, p2]);

if there is an error with one of the requests, the process ends in this scenario, we would like to make sure that responses are logged. however, there may be a small delay and the process terminates before the response is logged.

is there a way to wait until the interceptor has invoked the response hook for all pending requests?

kettanaito commented 2 months ago

Hi 👋

if there is an error with one of the requests, the process ends

That happens because you use Promise.all() though. Consider Promise.allSettled() if you expect request failures as well.

however, there may be a small delay and the process terminates before the response is logged.

Am I reading this right: the process exists before the "response" listeners you have are called?

The response event is emitted as soon as the response starts streaming. If your logic doesn't get called until the response finishes, I suspect you are doing something async in the response listener. Is that the case?

Overall, I don't think we should provide any means to await events as that's outside of tihs library's promise. But I'd like to learn more about your use case and see if I have something to recommend to achieve it.

kettanaito commented 2 months ago

Right now, we just emit the response event:

https://github.com/mswjs/interceptors/blob/8ba00cad812f50a489a16522f31a40af90fa86f1/src/interceptors/ClientRequest/NodeClientRequest.ts#L370

This means if the listener to that event is async, the process will not await its execution.

We can use the emitAsync() utility for the response event as well. That utility awaits all the listeners, even if they are async, to finish before freeing the event loop.

There are a couple of downsides to using it:

  1. It will be a part of the request resolution, so it will affect the request time.
  2. In some cases, like in NodeClientRequest, it will have no effect at all since ClientRequest.prototype.end must be synchronous. And there's nothing else to defer to the emitAsync() promise's completion.
mdesousa commented 2 months ago

hi @kettanaito thanks for your answer! you are correct, we are doing something async in the response listener. we are trying to log details about there response... and reading the body requires that we call await response.text(). see simplified code below:

  interceptor.on("response", async ({ response, requestId }) => {
    const { status } = response;
    const x = response.clone();
    const body = await x.text();
    console.debug(`res-${requestId}`, { status, body });
  });

below is an example to test the issue. when you run this, you'll see the "response completed" line prints before the console.debug called by the response listener. in cases where the process exists or throws before the response listener completes, nothing is logged. it would be nice if we had a way to wait for all listeners for the interceptor to settle.

const res = await fetch("https://cdn2.thecatapi.com/images/dh7.jpg");
console.log("response completed", res.status);
// if (!res.ok) throw... response body never gets logged
kettanaito commented 2 months ago

Yeah I think we have to await the response listeners. If you have none or all are sync, you pay nothing. If it's async, you expect that async to resolve within the request promise anyway.

Should be a matter of adding tests and swapping this.emitter.emit with emitAsync.

kettanaito commented 4 days ago

I've implemented this in https://github.com/mswjs/interceptors/pull/591. I will release this as the next minor version (breaking change) because it affects the resolution time of the responses.

kettanaito commented 3 days ago

Released: v0.31.0 🎉

This has been released in v0.31.0!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.