You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Create issues in the event that the body of the original response is cancelled with response.body.cancel(), if those clones are also not consumed. In such cases, the Promise returned by response.body.cancel() never settles (neither resolves nor rejects).
Further details
As best I can tell, this is due to the underlying .tee() call that happens on the ReadableStream that is the Response's body.
Particularly this bit from the docs made me wonder if the .clone() was the cause of our memory leak issues (doesn't look like it)
If only one cloned branch is consumed, then the entire body will be buffered in memory.
The behavior here though is really odd, and seems to indicate an actual bug in the implementation of ReadableStream (I think) in undici. What happens is that the Promise returned by response.body.cancel()never resolves or rejects if that response has previously been cloned, and the body of the clone has not yet been consumed.
Note
This behavior does not occur when tested in the browser. The .cancel() call resolves fairly quickly, and I assume the cloned response becomes disconnected from its source and eventually garbage collection.
A simple reproduction can be seen here:
import{setupServer}from'msw/node';if(process.env.ENABLE_MSW==='true'){constserver=setupServer();server.listen({onUnhandledRequest: 'bypass',});}asyncfunctionrun(){constresponse=awaitfetch('https://dummyjson.com/posts/1');awaitresponse.body.cancel();console.log('response body canceled');}run().then(()=>console.log('done')).catch((error)=>console.error('fail',error));
This program will produce no output at all with ENABLE_MSW=true, as the await response.body.cancel(); never settles. Incidentally it seems as though the underlying async operation is unref-ed as well, since it doesn't keep the event loop from terminating.
In our application, we got around this by patching the following in right after the emitAsync call:
const responseClone = response.clone()
await emitAsync(this.emitter, 'response', {
response: responseClone,
isMockedResponse: false,
request: requestCloneForResponseEvent,
requestId,
})
++ if (!responseClone.bodyUsed) {+ await responseClone.bytes(); // Consume the body if it hasn't been used+ }
This is a fairly brute-force approach and I'm not sure it would hold up in every scenario. But I thought I'd put it on our radar since in our case it caused server requests to never complete, creating 504 Gateway timeout errors in an application that simply called setupServer(), without registering any handlers.
This was a crazy rabbit-hole.
TL;DR:
The responses cloned here:
interceptors/src/interceptors/fetch/index.ts
Lines 161 to 169 in 77141e2
Create issues in the event that the
bodyof the originalresponseis cancelled withresponse.body.cancel(), if those clones are also not consumed. In such cases, thePromisereturned byresponse.body.cancel()never settles (neither resolves nor rejects).Further details
As best I can tell, this is due to the underlying
.tee()call that happens on theReadableStreamthat is the Response's body.Particularly this bit from the docs made me wonder if the
.clone()was the cause of our memory leak issues (doesn't look like it)The behavior here though is really odd, and seems to indicate an actual bug in the implementation of
ReadableStream(I think) inundici. What happens is that thePromisereturned byresponse.body.cancel()never resolves or rejects if that response has previously been cloned, and the body of the clone has not yet been consumed.Note
This behavior does not occur when tested in the browser. The
.cancel()call resolves fairly quickly, and I assume the cloned response becomes disconnected from its source and eventually garbage collection.A simple reproduction can be seen here:
This program will produce no output at all with
ENABLE_MSW=true, as theawait response.body.cancel();never settles. Incidentally it seems as though the underlying async operation is unref-ed as well, since it doesn't keep the event loop from terminating.In our application, we got around this by patching the following in right after the
emitAsynccall:const responseClone = response.clone() await emitAsync(this.emitter, 'response', { response: responseClone, isMockedResponse: false, request: requestCloneForResponseEvent, requestId, }) + + if (!responseClone.bodyUsed) { + await responseClone.bytes(); // Consume the body if it hasn't been used + }This is a fairly brute-force approach and I'm not sure it would hold up in every scenario. But I thought I'd put it on our radar since in our case it caused server requests to never complete, creating
504 Gatewaytimeout errors in an application that simply calledsetupServer(), without registering any handlers.