Request time out after 30 seconds

[lavrinec]

🐛 Bug Report

Hi, I found a bug with request timeout and I am not sure if it belongs here or in @elastic/transport. The problem is: if you set requestTimeout bigger than 30 seconds and pass node as a string in the new Client, you are ignored and your requests get timed out after 30 seconds with the exception message Request timed out.

To Reproduce

const client = new Client({ node: 'someUrl', requestTimeout: 60000 });
await client.bulk({ refresh: 'wait_for', operations }); // this code throws Request timed out if it runs more than 30s

Expected behavior

I would expect that I could have bulk requests longer than 30 seconds.

Problem explanation

When we are creating a new Client without an existing connection or connectionPool, then it gets created but with a max timeout of 30 seconds:

elasticsearch-js/src/client.ts: 231

this.connectionPool.addConnection(options.node ?? options.nodes)

addConnection accepts type AddConnectionOptions = string | ConnectionOptions; and if pass string then it will be transformed into URL and with it, a new UndiciConnection will be created.
UndiciConnection extends BaseConnection which at construction accept also timeout parameter which is then set to headersTimeout and bodyTimeout. If the timeout is not passed it uses the default 30 seconds.

Since the code in client.ts:231 does not pass object with the timeout parameter, it creates a connection with the default 30 seconds timeout.
So if we execute a call with a longer duration, we will receive the message Request timed out. despite we set the request timeout to 60 seconds. It does not help even if we set the requestTimeout parameter at the bulk function call.

Temporary workaround

const client = new Client({ node: {
                url: new URL('someUrl'),
                // @ts-ignore
                timeout: 60000,
            }, requestTimeout: 60000 });
await client.bulk({ refresh: 'wait_for', operations }); 

• node version: 14.19.2
• @elastic/elasticsearch version: ~8.4.0
• os: Mac

FYI: We did not get this error in version 7.17.

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove the stale label, or leave a comment, or this will be closed in 14 days.

[kaufmo]

@JoshMock do you have any news about this issue?

[a613]

Seems to do the trick & allow the requestTimeout passed to new Client() to be respected (valid for version 8.10.0):

diff --git a/node_modules/@elastic/elasticsearch/lib/client.js b/node_modules/@elastic/elasticsearch/lib/client.js
index 6952327..4dc718b 100644
--- a/node_modules/@elastic/elasticsearch/lib/client.js
+++ b/node_modules/@elastic/elasticsearch/lib/client.js
@@ -180,7 +180,13 @@ class Client extends api_1.default {
                 diagnostic: this.diagnostic,
                 caFingerprint: options.caFingerprint
             });
-            this.connectionPool.addConnection((_d = options.node) !== null && _d !== void 0 ? _d : options.nodes);
+            this.connectionPool.addConnection((_d = options.node) !== null && _d !== void 0 ? {
+                url: new URL(_d),
+                timeout: options.requestTimeout
+            } : {
+                ...options.nodes,
+                timeout: options.requestTimeout
+            });
         }
         this.transport = new options.Transport({
             diagnostic: this.diagnostic,

This can be patched using patch-package.

[minhhuynh-smg]

Do we have any update on the issue?

[JoshMock]

This is still a relevant issue. I've been on leave for the past 2 months, so am just getting back to work on this project.

[JoshMock]

A fix was merged in #2159, which will be available in 8.13 when it is released next week.