ClientRequest.destroy is not properly handled in Node 14 (breaks Got 11)

[mastermatt]

I was happy with the recent work that brought request.abort() inline with native Node (#1960). However, it turns out that in parallel, work was being done to deprecate abort in favor of destroy nodejs/node#33178.

💩

Quick summary of what I've figured out so far:
destroy has been a method on requests since v0.3, but it was undocumented. The idea being that a ClientRequest has an interface/strut of a Writeable Stream and therefor should have this method. In implementation, it was defined on the IncomingMessage class and just proxied the call to the socket, if it was already attached (source).
As of v14.1.0, destroy is now a first class method of ClientRequest and it handles several flows with and without a socket and emits several events on the request and socket when appropriate. ei it's now responsible for the "socket hang up" error. Meanwhile, abort has been pruned down to basically emit an aborted event then call destroy (source).

This is the core issue as to why Nock doesn't work with Got v11 #1984.
In what I'm sure will do down as an infamous PR, "Rewrite Got", began utilizing destroy for several error flows to ensure the response event doesn't get emitted. This doesn't seem to play well with the way Nock monkey patches abort but not destroy.

What changes need to happen:
I haven't gotten this far yet. Despite the name of this issue, monkey patching destroy may not be the right answer.
I'm going to do some more digging similar to how I approached the abort work, where I assert the events emitted by a request when abort is called at different places during a request's lifetime. This time I'll run it for Node 10, 12, and 14 both for abort and destroy.
One thing to note is that Node's codebase around these methods and events have been very active the last few months and I'm hesitant to put a lot of time into making Nock behave just like Node when it probably will change a bit before it goes LTS later this year. I think I'll focus more on Got since nothing they've changed should be Node 14 specific and Got 11 not working with Nock on Node 10-12 seems to be a short coming of Nock.

[mastermatt]

After a little more digging, I'm not sure the changes to abort/destroy in Node are related to why Got v11 isn't working. Got v11.1.0 is calling abort after receiving an error and seems to be unhappy when another error is emitted.

I'm going to keep looking into Got for now and if I can prove they're not related, I'll make a separate issue.
Currently, my leading hunch is that Got 11 has a regression that doesn't allow it handle multiple error events on the ClientRequest. I'll open an issue on Got's repo to flesh it out.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.