Allow passing in a signal to abort a cluster client request #37563

lukasolson · 2019-05-30T21:55:26Z

Resolves #37359.

This PR provides an additional optional option to the CallAPI options, signal, which represents a signal from an AbortController object. If provided, then controller.abort is invoked, then the request to ES itself will also be aborted.

Dev Docs

callWithRequest now accepts a AbortController signal for aborting requests to Elasticsearch. Example usage:

const { callWithRequest } = server.plugins.elasticsearch.getCluster('data');
const controller = new AbortController();
req.events.once('disconnect', () => controller.abort());
return await callWithRequest(req, 'search', params, { signal: controller.signal });

elasticmachine · 2019-05-30T21:55:28Z

Pinging @elastic/kibana-platform

elasticmachine · 2019-05-30T22:16:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

eliperelman · 2019-05-30T22:52:44Z

src/core/server/elasticsearch/cluster_client.ts

+        err ? reject(err) : resolve(res)
+      );
+      if (options.signal) {
+        options.signal.onabort = () => request.abort();


Not a requirement, but if you want to achieve this without mutating signal and therefore options, you can use:

options.signal.addEventListener('abort', () => request.abort());

eliperelman · 2019-05-30T22:53:37Z

src/core/server/elasticsearch/cluster_client.test.ts

+      signal: controller.signal,
+    });
+
+    expect(typeof controller.signal.onabort).toBe('function');


Instead of testing the AbortSignal itself, can we call abort and ensure the call fails?

elasticmachine · 2019-05-30T23:28:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

… cluster client

…e test

lukasolson · 2019-06-12T18:45:23Z

@eliperelman Could you give this another look?

elasticmachine · 2019-06-12T19:42:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-12T20:58:00Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-12T22:41:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Allow passing in a signal to abort an Elasticsearch request using the cluster client * Go back to using promises (which still return abort method) and update test * Update docs * Explicitly return Promise<any> instead of {}

lukasolson · 2019-06-12T22:47:24Z

7.x (7.3.0): 6ca6b1a

elasticmachine · 2019-07-09T17:45:10Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

sorenlouv · 2019-12-09T16:06:41Z

@lukasolson I've started seeing the following error on master and traced it back to here:

server    log   [09:53:55.180] [error][http] { Error: [security_exception] action [indices:data/read/search] is unauthorized for user [elastic]
    at respond (/home/apm/elastic/kibana/node_modules/elasticsearch/src/lib/transport.js:349:15)
    at checkRespForFailure (/home/apm/elastic/kibana/node_modules/elasticsearch/src/lib/transport.js:306:7)
    at HttpConnector.<anonymous> (/home/apm/elastic/kibana/node_modules/elasticsearch/src/lib/connectors/http.js:173:7)
    at IncomingMessage.wrapper (/home/apm/elastic/kibana/node_modules/elasticsearch/node_modules/lodash/lodash.js:4929:19)
    at IncomingMessage.elasticAPMCallbackWrapper (/home/apm/elastic/kibana/node_modules/elastic-apm-node/lib/instrumentation/index.js:309:27)
    at IncomingMessage.emit (events.js:194:15)
    at endReadableNT (_stream_readable.js:1103:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
  status: 403,
  displayName: 'AuthorizationException',
  message:
   '[security_exception] action [indices:data/read/search] is unauthorized for user [elastic]',
  path: '/apm-*%2Capm-*%2C%2Capm-*/_search',
  query: { terminate_after: 1, ignore_throttled: true },
  body:
   { error:
      { root_cause: [Array],
        type: 'security_exception',
        reason:
         'action [indices:data/read/search] is unauthorized for user [elastic]',
        caused_by: [Object] },
     status: 403 },
  statusCode: 403,
  response:
   '{"error":{"root_cause":[{"type":"security_exception","reason":"action [indices:data/read/search] is unauthorized for user [elastic]"}],"type":"security_exception","reason":"action [indices:data/read/search] is unauthorized for user [elastic]","caused_by":{"type":"string_index_out_of_bounds_exception","reason":"String index out of range: 0"}},"status":403}',
  toString: [Function],
  toJSON: [Function] }
server   error  [09:53:55.047]  Error: Internal Server Error
    at HapiResponseAdapter.toInternalError (/home/apm/elastic/kibana/src/core/server/http/router/response_adapter.ts:55:19)
    at Router.handle (/home/apm/elastic/kibana/src/core/server/http/router/router.ts:297:34)
    at process._tickCallback (internal/process/next_tick.js:68:7)
TypeError: request.abort is not a function
    at AbortSignal.options.signal.addEventListener (/home/apm/elastic/kibana/src/core/server/elasticsearch/cluster_client.ts:67:19)
    at Timeout._onTimeout (/home/apm/elastic/kibana/node_modules/abortcontroller-polyfill/dist/cjs-ponyfill.js:153:27)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)
 server crashed  with status code 1

There are obviously some auth issues but the thing that makes the server crash is the request.abort is not a function error. Any idea?

lukasolson · 2019-12-09T17:57:03Z

Hmm, the object returned from calling the Elasticsearch JS client should always be returning something with an abort method (or so I thought). I wonder if the fact that the request is failing has something to do with why it's not there. Do you only get this with the associated auth issues?

sorenlouv · 2019-12-09T22:00:33Z

Do you only get this with the associated auth issues?

It looks like it, yeah.

Since error tracking is now enabled in Kibana (opt-in) I was able to find the error with a bit more context. Does this give you anything?

sorenlouv · 2019-12-10T11:21:49Z

@lukasolson Turns out this is caused by the APM node agent: #52617.
Sorry for the noise.

lukasolson added review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels May 30, 2019

lukasolson requested review from azasypkin and joshdover May 30, 2019 21:55

lukasolson requested a review from a team as a code owner May 30, 2019 21:55

lukasolson self-assigned this May 30, 2019

lukasolson added v7.3.0 v8.0.0 labels May 30, 2019

lukasolson force-pushed the clusterClientAbort branch from 0ccd052 to 812b644 Compare May 30, 2019 22:29

eliperelman reviewed May 30, 2019

View reviewed changes

lukasolson mentioned this pull request Jun 12, 2019

[skip ci] Data access service #38737

Closed

4 tasks

lukasolson added 2 commits June 12, 2019 10:22

Allow passing in a signal to abort an Elasticsearch request using the…

4427234

… cluster client

Go back to using promises (which still return abort method) and updat…

40cfb39

…e test

lukasolson force-pushed the clusterClientAbort branch from 812b644 to 40cfb39 Compare June 12, 2019 18:44

Update docs

49a60cb

eliperelman approved these changes Jun 12, 2019

View reviewed changes

Explicitly return Promise<any> instead of {}

d519962

lukasolson merged commit 9e472e6 into elastic:master Jun 12, 2019

lukasolson deleted the clusterClientAbort branch June 12, 2019 22:45

joshdover added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Jul 9, 2019

lukasolson mentioned this pull request Aug 6, 2019

Abort request to Elasticsearch when browser disconnects #42764

Merged

1 task

lukasolson mentioned this pull request Sep 26, 2019

Refactor, deangularize courier/search source #45235

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow passing in a signal to abort a cluster client request #37563

Allow passing in a signal to abort a cluster client request #37563

lukasolson commented May 30, 2019 •

edited by joshdover

Loading

elasticmachine commented May 30, 2019

elasticmachine commented May 30, 2019

eliperelman May 30, 2019

eliperelman May 30, 2019

elasticmachine commented May 30, 2019

lukasolson commented Jun 12, 2019

elasticmachine commented Jun 12, 2019

elasticmachine commented Jun 12, 2019

elasticmachine commented Jun 12, 2019

lukasolson commented Jun 12, 2019

elasticmachine commented Jul 9, 2019

sorenlouv commented Dec 9, 2019

lukasolson commented Dec 9, 2019

sorenlouv commented Dec 9, 2019

sorenlouv commented Dec 10, 2019

Allow passing in a signal to abort a cluster client request #37563

Allow passing in a signal to abort a cluster client request #37563

Conversation

lukasolson commented May 30, 2019 • edited by joshdover Loading

Dev Docs

elasticmachine commented May 30, 2019

elasticmachine commented May 30, 2019

💔 Build Failed

eliperelman May 30, 2019

Choose a reason for hiding this comment

eliperelman May 30, 2019

Choose a reason for hiding this comment

elasticmachine commented May 30, 2019

💔 Build Failed

lukasolson commented Jun 12, 2019

elasticmachine commented Jun 12, 2019

💔 Build Failed

elasticmachine commented Jun 12, 2019

💔 Build Failed

elasticmachine commented Jun 12, 2019

💚 Build Succeeded

lukasolson commented Jun 12, 2019

elasticmachine commented Jul 9, 2019

💚 Build Succeeded

sorenlouv commented Dec 9, 2019

lukasolson commented Dec 9, 2019

sorenlouv commented Dec 9, 2019

sorenlouv commented Dec 10, 2019

lukasolson commented May 30, 2019 •

edited by joshdover

Loading