Skip to content

Commit cebe956

Browse files
[Backport 8.13] Ensure new connections inherit client's set defaults (#2163)
* Add test confirming the issue See #1791 * fix: ensure new connections inherit the client instance's defaults for #1791 (cherry picked from commit 05e3139) Co-authored-by: Josh Mock <[email protected]>
1 parent dbb685f commit cebe956

File tree

2 files changed

+73
-4
lines changed

2 files changed

+73
-4
lines changed

src/client.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import {
4444
Context
4545
} from '@elastic/transport/lib/types'
4646
import { RedactionOptions } from '@elastic/transport/lib/Transport'
47-
import BaseConnection, { prepareHeaders } from '@elastic/transport/lib/connection/BaseConnection'
47+
import BaseConnection, { prepareHeaders, ConnectionOptions } from '@elastic/transport/lib/connection/BaseConnection'
4848
import SniffingTransport from './sniffingTransport'
4949
import Helpers from './helpers'
5050
import API from './api'
@@ -237,7 +237,35 @@ export default class Client extends API {
237237
diagnostic: this.diagnostic,
238238
caFingerprint: options.caFingerprint
239239
})
240-
this.connectionPool.addConnection(options.node ?? options.nodes)
240+
241+
// ensure default connection values are inherited when creating new connections
242+
// see https://github.com/elastic/elasticsearch-js/issues/1791
243+
const nodes = options.node ?? options.nodes
244+
let nodeOptions: Array<string | ConnectionOptions> = Array.isArray(nodes) ? nodes : [nodes]
245+
type ConnectionDefaults = Record<string, any>
246+
nodeOptions = nodeOptions.map(opt => {
247+
const { tls, headers, auth, requestTimeout: timeout, agent, proxy, caFingerprint } = options
248+
let defaults: ConnectionDefaults = { tls, headers, auth, timeout, agent, proxy, caFingerprint }
249+
250+
// strip undefined values from defaults
251+
defaults = Object.keys(defaults).reduce((acc: ConnectionDefaults, key) => {
252+
const val = defaults[key]
253+
if (val !== undefined) acc[key] = val
254+
return acc
255+
}, {})
256+
257+
let newOpts
258+
if (typeof opt === 'string') {
259+
newOpts = {
260+
url: new URL(opt)
261+
}
262+
} else {
263+
newOpts = opt
264+
}
265+
266+
return { ...defaults, ...newOpts }
267+
})
268+
this.connectionPool.addConnection(nodeOptions)
241269
}
242270

243271
this.transport = new options.Transport({
@@ -282,7 +310,7 @@ export default class Client extends API {
282310
// Merge the new options with the initial ones
283311
// @ts-expect-error kChild symbol is for internal use only
284312
const options: ClientOptions = Object.assign({}, this[kInitialOptions], opts)
285-
// Pass to the child client the parent instances that cannot be overriden
313+
// Pass to the child client the parent instances that cannot be overridden
286314
// @ts-expect-error kInitialOptions symbol is for internal use only
287315
options[kChild] = {
288316
connectionPool: this.connectionPool,

test/unit/client.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
* under the License.
1818
*/
1919

20+
import * as http from 'http'
2021
import { test } from 'tap'
2122
import { URL } from 'url'
22-
import { connection } from '../utils'
23+
import FakeTimers from '@sinonjs/fake-timers'
24+
import { buildServer, connection } from '../utils'
2325
import { Client, errors } from '../..'
2426
import * as symbols from '@elastic/transport/lib/symbols'
2527
import { BaseConnectionPool, CloudConnectionPool, WeightedConnectionPool } from '@elastic/transport'
@@ -441,3 +443,42 @@ test('user agent is in the correct format', t => {
441443
t.ok(/^\d+\.\d+\.\d+/.test(agentSplit[0].split('/')[1]))
442444
t.end()
443445
})
446+
447+
test('Ensure new client instance stores requestTimeout for each connection', t => {
448+
const client = new Client({
449+
node: { url: new URL('http://localhost:9200') },
450+
requestTimeout: 60000,
451+
})
452+
t.equal(client.connectionPool.connections[0].timeout, 60000)
453+
t.end()
454+
})
455+
456+
test('Ensure new client does not time out at default (30s) when client sets requestTimeout', async t => {
457+
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
458+
t.teardown(() => clock.uninstall())
459+
460+
function handler (_req: http.IncomingMessage, res: http.ServerResponse) {
461+
setTimeout(() => {
462+
t.pass('timeout ended')
463+
res.setHeader('content-type', 'application/json')
464+
res.end(JSON.stringify({ success: true }))
465+
}, 31000) // default is 30000
466+
clock.runToLast()
467+
}
468+
469+
const [{ port }, server] = await buildServer(handler)
470+
471+
const client = new Client({
472+
node: `http://localhost:${port}`,
473+
requestTimeout: 60000
474+
})
475+
476+
try {
477+
await client.transport.request({ method: 'GET', path: '/' })
478+
} catch (error) {
479+
t.fail('timeout error hit')
480+
} finally {
481+
server.stop()
482+
t.end()
483+
}
484+
})

0 commit comments

Comments
 (0)