Skip to content

Commit

Permalink
Ensure new connections inherit client's set defaults (#2159)
Browse files Browse the repository at this point in the history
* Add test confirming the issue

See #1791

* fix: ensure new connections inherit the client instance's defaults

for #1791

(cherry picked from commit 05e3139)
  • Loading branch information
JoshMock authored and github-actions[bot] committed Mar 20, 2024
1 parent 7471710 commit 9ca2c2f
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 4 deletions.
34 changes: 31 additions & 3 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
Context
} from '@elastic/transport/lib/types'
import { RedactionOptions } from '@elastic/transport/lib/Transport'
import BaseConnection, { prepareHeaders } from '@elastic/transport/lib/connection/BaseConnection'
import BaseConnection, { prepareHeaders, ConnectionOptions } from '@elastic/transport/lib/connection/BaseConnection'
import SniffingTransport from './sniffingTransport'
import Helpers from './helpers'
import API from './api'
Expand Down Expand Up @@ -237,7 +237,35 @@ export default class Client extends API {
diagnostic: this.diagnostic,
caFingerprint: options.caFingerprint
})
this.connectionPool.addConnection(options.node ?? options.nodes)

// ensure default connection values are inherited when creating new connections
// see https://github.com/elastic/elasticsearch-js/issues/1791
const nodes = options.node ?? options.nodes
let nodeOptions: Array<string | ConnectionOptions> = Array.isArray(nodes) ? nodes : [nodes]
type ConnectionDefaults = Record<string, any>
nodeOptions = nodeOptions.map(opt => {
const { tls, headers, auth, requestTimeout: timeout, agent, proxy, caFingerprint } = options
let defaults: ConnectionDefaults = { tls, headers, auth, timeout, agent, proxy, caFingerprint }

// strip undefined values from defaults
defaults = Object.keys(defaults).reduce((acc: ConnectionDefaults, key) => {
const val = defaults[key]
if (val !== undefined) acc[key] = val
return acc
}, {})

let newOpts
if (typeof opt === 'string') {
newOpts = {
url: new URL(opt)
}
} else {
newOpts = opt
}

return { ...defaults, ...newOpts }
})
this.connectionPool.addConnection(nodeOptions)
}

this.transport = new options.Transport({
Expand Down Expand Up @@ -282,7 +310,7 @@ export default class Client extends API {
// Merge the new options with the initial ones
// @ts-expect-error kChild symbol is for internal use only
const options: ClientOptions = Object.assign({}, this[kInitialOptions], opts)
// Pass to the child client the parent instances that cannot be overriden
// Pass to the child client the parent instances that cannot be overridden
// @ts-expect-error kInitialOptions symbol is for internal use only
options[kChild] = {
connectionPool: this.connectionPool,
Expand Down
43 changes: 42 additions & 1 deletion test/unit/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
* under the License.
*/

import * as http from 'http'
import { test } from 'tap'
import { URL } from 'url'
import { connection } from '../utils'
import FakeTimers from '@sinonjs/fake-timers'
import { buildServer, connection } from '../utils'
import { Client, errors } from '../..'
import * as symbols from '@elastic/transport/lib/symbols'
import { BaseConnectionPool, CloudConnectionPool, WeightedConnectionPool } from '@elastic/transport'
Expand Down Expand Up @@ -441,3 +443,42 @@ test('user agent is in the correct format', t => {
t.ok(/^\d+\.\d+\.\d+/.test(agentSplit[0].split('/')[1]))
t.end()
})

test('Ensure new client instance stores requestTimeout for each connection', t => {
const client = new Client({
node: { url: new URL('http://localhost:9200') },
requestTimeout: 60000,
})
t.equal(client.connectionPool.connections[0].timeout, 60000)
t.end()
})

test('Ensure new client does not time out at default (30s) when client sets requestTimeout', async t => {
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
t.teardown(() => clock.uninstall())

function handler (_req: http.IncomingMessage, res: http.ServerResponse) {
setTimeout(() => {
t.pass('timeout ended')
res.setHeader('content-type', 'application/json')
res.end(JSON.stringify({ success: true }))
}, 31000) // default is 30000
clock.runToLast()
}

const [{ port }, server] = await buildServer(handler)

const client = new Client({
node: `http://localhost:${port}`,
requestTimeout: 60000
})

try {
await client.transport.request({ method: 'GET', path: '/' })
} catch (error) {
t.fail('timeout error hit')
} finally {
server.stop()
t.end()
}
})

0 comments on commit 9ca2c2f

Please sign in to comment.