Always add a on(error) handler on sockets

[mcollina]

I couldn't reproduce the problem, but this might be the cause of #3848.

@ggoodman can you test this PR?

[ggoodman]

Gimme a sec to put this to the test.

[ggoodman]

I've tried that out and continue to see the process-level crash. I believe that a naive fix is the following:

diff --git a/lib/api/api-request.js b/lib/api/api-request.js
index 9ae7ed6c..a5e28834 100644
--- a/lib/api/api-request.js
+++ b/lib/api/api-request.js
@@ -110,6 +110,8 @@ class RequestHandler extends AsyncResource {
       highWaterMark
     })
 
+    res.on('error', noop)
+
     if (this.removeAbortListener) {
       res.on('close', this.removeAbortListener)
       this.removeAbortListener = null

[ggoodman]

An alternative that would continue to allow "error" events on res.body to cause uncaught exceptions (except for the critical window in which userspace doesn't have a reference to the res might look like this:

diff --git a/lib/api/api-request.js b/lib/api/api-request.js
index 9ae7ed6c..53793525 100644
--- a/lib/api/api-request.js
+++ b/lib/api/api-request.js
@@ -177,7 +177,15 @@ function request (opts, callback) {
   if (callback === undefined) {
     return new Promise((resolve, reject) => {
       request.call(this, opts, (err, data) => {
-        return err ? reject(err) : resolve(data)
+        if (err) {
+          return reject(err)
+        }
+
+        data.body.on('error', noop)
+
+        queueMicrotask(() => data.body.off('error', noop))
+
+        return resolve(data)
       })
     })
   }

This is untested. Not sure if the ordering works out like we'd want. Might have to call queueMicrotask after calling resolve... 🤔

EDIT: This did not work.