req.abort() with a body that is a stream calls callback(err) twice

[trentm]

🐛 Bug Report

When (a) using a body that is a stream to client.search(), and (b) calling req.abort(), the callback is called twice. Once for the RequestAbortedError, as expected, and once for a "premature close" error from end-of-stream, used by pump, used by the client.

To Reproduce

const { Readable } = require('stream')
const { Client } = require('@elastic/elasticsearch')

var client = new Client({ node: 'http://localhost:9200' })

// A "body" stream that takes 1s to produce the content.
const slowBody = new Readable({
  read(size) {
    setTimeout(() => {
      this.push('{"size":1, "query":{"match_all":{}}}')
      this.push(null) // EOF
    }, 1000).unref()
  }
})

// Aborting a request with a *stream* body will result in the callback being
// called *twice*:
// - once for the RequestAbortError, as expected, and
// - once for a "premature close" error from end-of-stream, used by pump,
//   used by the client
let req = client.search(
  {body: slowBody},
  function (err, result) {
    if (err) {
      console.log('err:', err)
    } else if (result) {
      console.log('result hits:', result.body.hits.hits)
    }
  }
)
setImmediate(() => {
  req.abort()
})

Result:

% node double-callback.js
err: RequestAbortedError: Request aborted
    at ClientRequest.onAbort (/Users/trentm/tmp/b/node_modules/@elastic/elasticsearch/lib/Connection.js:122:16)
    at ClientRequest.emit (events.js:327:22)
    at emitAbortNT (_http_client.js:395:7)
    at processTicksAndRejections (internal/process/task_queues.js:79:21) {
  meta: {
    body: null,
    statusCode: null,
    headers: null,
    meta: {
      context: null,
      request: [Object],
      name: 'elasticsearch-js',
      connection: [Object],
      attempts: 0,
      aborted: true
    }
  }
}
err: Error: premature close
    at onclosenexttick (/Users/trentm/tmp/b/node_modules/end-of-stream/index.js:54:86)
    at processTicksAndRejections (internal/process/task_queues.js:75:11) {
  meta: {
    body: null,
    statusCode: null,
    headers: null,
    meta: {
      context: null,
      request: [Object],
      name: 'elasticsearch-js',
      connection: [Object],
      attempts: 0,
      aborted: true
    }
  }
}

Expected behavior

I expect to have callback(err) only called once (with the RequestAbortedError).

Your Environment

• node version: 10, 14
• @elastic/elasticsearch version: @elastic/elasticsearch@7.10.0
• os: Mac

[trentm]

Here is a possible (and possibly naive) fix: https://github.com/elastic/elasticsearch-js/compare/master...trentm:fix-double-callback?w=1

Let me know if you think that approach is reasonable and would like a PR.

[delvedor]

Confirmed, thanks for reporting! I'll work on a fix :)