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
I found the existing answers insufficient on this point so I thought I would try to write up an explanation.
By default request will follow up to ten redirects. If you have a server that is misbehaving, for example redirecting to itself, request will follow the redirect up to ten times and then return an error.
However, the request also listens for an incoming request body - you can pipe a Readable to the request object and this library will read and upload that response. This is done by registering a request.on('pipe', (src) => ...) listener in the request source code.
The problem is that on each followed redirect, request redeclares the .on('pipe', (src) => ...) listener, so by the 7th redirect you have 7 listeners and so on. By default the EventEmitter warns after ten of the same event have been registered, which is why you get Possible memory leak detected exactly once, with the default settings for both the EventEmitter and for request.
This is probably a mistake - if you did manage to pipe the request body to the 7th HTTP request, you would have seven listeners by that point, each firing independently, and it's not clear what would happen - you might end up with the request body duplicated seven times, or just written seven times to something that's not listening. However, there is a separate issue where this piped request body is only sent on the first request, not all of them, see #3138.
So is there actually a memory leak if you have lots of redirects? Probably not... the listeners are registered on the request object, as long as that request object goes out of scope (you're not holding a reference to it somewhere) then it will eventually get garbage collected and the event emitters will be collected.
What is the fix? In this library, either unregister and re-register the pipe handler each time a new redirect is issued, don't re-register the pipe handler if we are following a redirect.
In your code, it's probably safe to either set maxRedirects to 9, or EventEmitter default to 11, either of which will resolve the issue, as long as your request objects are getting garbage collected (they probably are).
Note if you are getting memory leak detected for behavior other than redirect following there may still be a memory leak, see for example #2575.
The text was updated successfully, but these errors were encountered:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
I found the existing answers insufficient on this point so I thought I would try to write up an explanation.
By default
request
will follow up to ten redirects. If you have a server that is misbehaving, for example redirecting to itself, request will follow the redirect up to ten times and then return an error.However, the request also listens for an incoming request body - you can pipe a Readable to the
request
object and this library will read and upload that response. This is done by registering arequest.on('pipe', (src) => ...)
listener in the request source code.The problem is that on each followed redirect,
request
redeclares the.on('pipe', (src) => ...)
listener, so by the 7th redirect you have 7 listeners and so on. By default the EventEmitter warns after ten of the same event have been registered, which is why you get Possible memory leak detected exactly once, with the default settings for both the EventEmitter and forrequest
.This is probably a mistake - if you did manage to pipe the request body to the 7th HTTP request, you would have seven listeners by that point, each firing independently, and it's not clear what would happen - you might end up with the request body duplicated seven times, or just written seven times to something that's not listening. However, there is a separate issue where this piped request body is only sent on the first request, not all of them, see #3138.
So is there actually a memory leak if you have lots of redirects? Probably not... the listeners are registered on the request object, as long as that request object goes out of scope (you're not holding a reference to it somewhere) then it will eventually get garbage collected and the event emitters will be collected.
What is the fix? In this library, either unregister and re-register the pipe handler each time a new redirect is issued, don't re-register the pipe handler if we are following a redirect.
In your code, it's probably safe to either set maxRedirects to 9, or EventEmitter default to 11, either of which will resolve the issue, as long as your
request
objects are getting garbage collected (they probably are).Note if you are getting
memory leak detected
for behavior other than redirect following there may still be a memory leak, see for example #2575.The text was updated successfully, but these errors were encountered: