use FinalizationRegistry to cancel the body if response is collected

[mcollina]

Fixes #2856

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 97.72727% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 94.17%. Comparing base (ee64eb5) to head (728412f).
Report is 1 commits behind head on main.

Files	Patch %	Lines
lib/web/fetch/response.js	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3199      +/-   ##
==========================================
- Coverage   94.19%   94.17%   -0.03%     
==========================================
  Files          90       90              
  Lines       24493    24550      +57     
==========================================
+ Hits        23071    23119      +48     
- Misses       1422     1431       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mcollina]

@KhafraDev I'm getting 8 WPT unexpected failures:

Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:339:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:228:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:228:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:248:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:228:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:228:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:228:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "/Users/matteo/Repositories/undici/test/wpt/tests/fetch/api/abort/general.any.js":
AbortError: This operation was aborted
    at new DOMException (node:internal/per_context/domexception:53:5)
    at AbortController.abort (node:internal/abort_controller:392:18)
    at Test.<anonymous> (evalmachine.<anonymous>:376:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

I'm not sure how to debug this. Can you help?

[KhafraDev]

Weird test, I don't totally understand what's happening:

import { fetch } from './index.js'
import { createServer } from 'node:http'
import { once } from 'node:events'

process.on('uncaughtException', (e) => console.log('uncaught', e))

const server = createServer((req, res) => {
  setInterval(() => res.write(Buffer.from('hello, world')), 1)
}).listen(0)

await once(server, 'listening')

const ac = new AbortController()

await fetch(`http://localhost:${server.address().port}`, { signal: ac.signal })
ac.abort()

while (true) {
  await fetch(`http://localhost:${server.address().port}`).then(r => r.text())
}

[mcollina]

Found it!

[ronag]

I'm thinking could we do this one level lower? e.g. if the handler or resume is no longer referenced?

[ronag]

Instead of relying on finalization registry + GC. Should we have a setInterval loop that check all references once a second or something?

[metcoder95]

added

This can potentially benefit several use cases where there are side effects of not consuming the body, making the flow easier to reason about

[mcollina]

@KhafraDev I think this is good to go now.

[Uzlopak]

ok, so the changes by @tsctx got reverted?

[mcollina]

I've likely made a mess with the merge.

[mcollina]

@KhafraDev @Uzlopak @ronag PTAL