fix(core): End node:http client spans when the response is destroyed

[4ktLuffy]

When the caller destroys an http.get() response mid-body with res.destroy() (no error), the http.client span is never ended, so it is never sent. The response emits only close, not end or error, and the request close fallback is removed once the response arrives.

This adds a response close listener that ends the span. Following #25260, a destroy by the caller is not treated as a failure: the span keeps the status of the response (ok for a 200, error for a 404). res.destroy(err) and a server closing the connection mid-body still set an error status, because they emit error before close. After a normal end or an error, the new listener is a no-op.

Tests: caller destroy (200 and 404), caller destroy with an error, and a new server-abort case. The caller-destroy test times out without the fix. All 121 http tests pass. Reproduced on @sentry/node 11.8.0.

Closes #25469

[s1gr1d]

Thanks for the contribution, I have some comments on it.

btw, also linking this PR for reference: #25260

[s1gr1d]

close is not only emitted when the caller destroys the response, but also when the connection dies/resets (server reset, req.destroy()).

Node calls res.destroy(ECONNRESET) and IncomingMessage only emits error if there is an 'error' listener (but it does not count errorMonitor).

If user code does not listen for error on the response, the span ends as ok (common with http.get(url, res => res.pipe(...))).

response.errored separates the two cases. It is null after res.destroy() and holds the error after a reset, even when Node did not emit that error:

response.on('close', () => finishWithResponse(response.errored));

With this change, you can also remove the destroyedByCaller flag. Could you check that?

[s1gr1d]

If you use response.errored, you can remove the destroyedByCaller parameter.

[s1gr1d]

Please make the comment shorter and correct it after the change, for example: "res.destroy() emits only close. errored separates a caller destroy from a connection error that was not emitted because nothing listens for error." Right now it says that close always means a caller destroy, which is not true.

[4ktLuffy]

Thanks, you were right. I reproduced it: with no error listener, a reset only emits close with errored set, and the span ended as ok. Pushed the fix:

• close now calls finishWithResponse(response.errored), and destroyedByCaller is gone.
• Also removed the aborted check, since Node sets aborted on a plain res.destroy() too.
• Added errored to HttpIncomingMessage for the type check.
• Tests now use the real errorMonitor (the old default added an error listener and hid the bug), plus a new test for a reset with no listener.