ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.57k stars 1.33k forks source link

fix: do not force-set `req.destroyed = true` on abort #1774

Closed afharo closed 10 months ago

afharo commented 10 months ago

The current this.req.destroyed = true hack that was added in https://github.com/ladjs/superagent/commit/8b1923d4a0f22bec49ec6eebead499ce0ac7f694 actually breaks the behavior of 14.0.0.

Looking at v14.0.0's HTTP client's code, the abort method internally calls destroy.

destroy has an early termination when destroyed is already true. Setting destroyed = true before calling abort() stops Node.js from internally closing the socket, potentially leading to memory leaks.

It also breaks the behavior of abort() since Node.js deprecated it in favor of destroy() (as Node.js internally relies on destroy() logic to terminate the call.

Checklist

titanism commented 10 months ago

v8.1.0 released which fixes this issue, thank you

npm install superagent@8.1.0

release notes @ https://github.com/ladjs/superagent/releases/tag/v8.1.0