HttpConnection.buildRequestObject includes non-http.request fields, which breaks in recent node v20 nightly

[trentm]

🐛 Bug Report

When using the HttpConnection option with the ES client, and using the latest node.js v20 nightly, HTTP communication is broken.

nodejs/node#46904 recently (first in v20.0.0-nightly2023030448f99e5f6a nightly Node.js build) introduced a change to the internal isURL function that is used by http.request() and https.request() to decide if the first argument is a URL instance. The new test is if the object includes href and origin fields. If that is true, then Node's http.request() takes this code path which results in other fields like headers, agent, etc. being ignored.

HttpConnection.buildRequestObject hits this issue:

elastic-transport-js/src/connection/HttpConnection.ts

Lines 344 to 345 in 64d6fa2

	href: url.href,
	origin: url.origin,

(I've opened issue nodejs/node#46981 on Node.js to discuss the change, but either way, I think this transport should change to only pass documented http.request() options.)

To Reproduce

// es-nodev20-break2.example.js
const es = require('@elastic/elasticsearch')

const http = require('http')

// A mock ES server that responds with a valid response for a search request.
const server = http.createServer((req, res) => {
  console.log('SERVER: on request: %s %s %s', req.method, req.url, req.headers)
  req.resume()
  req.on('end', () => {
    res.writeHead(200, {
      'X-elastic-product': 'Elasticsearch',
      'content-type': 'application/json'
    })
    const body = '{"took":0,"timed_out":false,"_shards":{"total":0,"successful":0,"skipped":0,"failed":0},"hits":{"total":{"value":0,"relation":"eq"},"max_score":0,"hits":[]}}'
    res.end(body)
  })
})

server.listen(0, '127.0.0.1', () => {
  const serverUrl = `http://127.0.0.1:${server.address().port}`
  const client = new es.Client({
    Connection: es.HttpConnection,
    node: serverUrl
  })
  client.search({ q: 'pants' })
    .then(() => {
      console.log('CLIENT: search success')
    })
    .finally(() => {
      client.close()
      server.close()
    })
})

Expected behavior

When run with the v20.0.0-nightly2023030448f99e5f6a node nightly, note that the custom headers from the ES client do not get through to the server:

% node --version
v20.0.0-nightly2023030448f99e5f6a

% node es-nodev20-break2.example.js
SERVER: on request: GET /_search?q=pants { host: '127.0.0.1:65287', connection: 'keep-alive' }
CLIENT: search success

But when run with earlier node.js versions, they do:

% nvm use v20.0.0-nightly20230303a37b72da87
Now using node v20.0.0-nightly20230303a37b72da87 (npm v9.5.1)

% node es-nodev20-break2.example.js
SERVER: on request: GET /_search?q=pants {
  'user-agent': 'elastic-transport-js/8.3.1 (darwin 22.3.0-x64; Node.js v20.0.0-nightly20230303a37b72da87)',
  'x-elastic-client-meta': 'es=8.6.0,js=20.0.0-nightly20230303a37b72da87,t=8.3.1,hc=20.0.0-nightly20230303a37b72da87',
  accept: 'application/vnd.elasticsearch+json; compatible-with=8,text/plain',
  host: '127.0.0.1:65289',
  connection: 'keep-alive'
}
CLIENT: search success

npm test also fails with that node version.

Your Environment

• node version: v20.0.0-nightly2023030448f99e5f6a
• @elastic/elasticsearch version: >=8.0.0
• os: Mac, though this will happen on any platform
• any other relevant information

[trentm]

I've opened issue nodejs/node#46981 on Node.js to discuss the change,

Since then, node core has effectively fixed this issue in nodejs/node#46989

If you now test with a node v20 nightly after that change, the tests pass:

% NVM_NODEJS_ORG_MIRROR=https://nodejs.org/download/nightly nvm install v20.0.0-nightly2023032738e6ac7b44
...

% node --version
v20.0.0-nightly2023032738e6ac7b44

% npm test
...

but either way, I think this transport should change to only pass documented http.request() options.

I'll open a PR to do this.