[BUG] Cannot destructure property 'body' in the bulk helper after result has been returned to the caller

[sebekz]

What is the bug? I am getting an error when trying to use the client.helpers.bulk helper.

TypeError: Cannot destructure property 'body' of 'undefined' as it is undefined.
      at node_modules/@opensearch-project/opensearch/lib/Helpers.js:704:93

How can one reproduce the bug?

I created an isolated test to illustrate the issue. Hopefully it's easily reproducible this way.

import { Client } from '@opensearch-project/opensearch';
import * as AWS from 'aws-sdk';

// My fork of https://www.npmjs.com/package/aws-elasticsearch-connector capable of signing requests to AWS OpenSearch
// @opensearch-project/opensearch is not yet capable of signing AWS requests
const createAwsElasticsearchConnector = require('../modules/aws-oss-connector');

const domain =
  'PUT_YOUR_DOMAIN_URL_HERE.es.amazonaws.com';
const index = 'YOUR_TEST_INDEX_NAME';

const bootstrapOSSClient = (): Client => {
  const ossConnectorConfig = createAwsElasticsearchConnector(AWS.config);
  const client = new Client({
    ...ossConnectorConfig,
    node: `https://${domain}`,
  });

  return client;
};

const main = async (): Promise<void> => {
  try {
    console.info('Starting processing');

    // TEST DEFINITION
    const input = [
      { id: '1', name: 'test' },
      { id: '2', name: 'test 2' },
    ];

    const client = bootstrapOSSClient();

    const response = await client.helpers.bulk({
      datasource: input,
      onDocument(doc: any) {
        console.info(`Processing document #${doc.id}`);
        return {
          index: { _index: index, _id: doc.id },
        };
      },
    });

    console.info(`Indexed ${response.successful} documents`);

    // END TEST DEFINITION

    console.info('Finished processing');
  } catch (error) {
    console.warn(`Error in main(): ${error}`);
  }
};

try {
  main().then(() => {
    console.info('Exited main()');
  });
} catch (error) {
  console.warn(`Top-level error: ${error}`);
}

and the result was

$ npx ts-node ./.vscode/test.ts
Starting processing
Processing document #1
Processing document #2
(node:39232) UnhandledPromiseRejectionWarning: TypeError: Cannot destructure property 'body' of 'undefined' as it is undefined.
    at D:\Development\eSUB\Coronado\git\platform\node_modules\@opensearch-project\opensearch\lib\Helpers.js:704:93
(Use `node --trace-warnings ...` to show where the warning was created)
(node:39232) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:39232) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Indexed 2 documents
Finished processing
Exited main()

Stepping through the code I am able to intercept a single call to node_modules/@opensearch-project/opensearch/lib/Helpers.js:704:93 where

• client.bulk() is called
• which calls bulkApi() in \opensearch\api\api\bulk.js and returns successfully
• await finish() is called in \opensearch\lib\Helpers.js:559
• inside \opensearch\lib\Transport.js a call to prepareRequest() is made, ending with return transportReturn
• this ends up in request():177 calling

return p.then(onFulfilled, onRejected)

with p being null at that time. That resulted my callback in the AWS Transport class responsible for signing requests to callback to the Helper.js tryBulk() with second parameter undefined, resulting in the Cannot destructure property 'body' error.

What is the expected behavior? Transport.js implementation of the request does not result in a null p promise issue when callback is passed in the request() call

What is your host/environment?

• VS Code + Node v14 + ES5
• Connecting to OSS 1.0 instance in AWS

Do you have any additional context? Looks to be identical as this SO unresolved issue

[kavilla]

Hello @sebekz, thanks for opening this. Quick question: are you running from the released NPM package or running from source?

[sebekz]

Hey @kavilla. I am running from npm @opensearch-project/opensearch latest official 1.0.0 build.

[kavilla]

Gotcha, right now the latest official build is missing OpenSearchDashboardsClient but I am not positive if its the same issue here: https://github.com/opensearch-project/opensearch-js/issues/181. It got missed in the npm ignore filter.

I am hesitate about diving too deep until that gets officially released. @ananzh, should we prioritize releasing the fix?

[apostrophest]

@kavilla This is not related to https://github.com/opensearch-project/opensearch-js/issues/181

This is a long-standing bug in the callback handling of Elastic's JavaScript client library. This is an inappropriate destructuring https://github.com/elastic/elasticsearch-js/blob/v7.10.0/lib/Helpers.js#L679 because in the case of an error, you'd expect the first argument to be called with an Error and the second argument to be null/undefined/unspecified. Destructuring is nice, but it's an overeager use of that feature. This bug has been here since Elastic's introduction of the bulk helper. https://github.com/elastic/elasticsearch-js/commit/51169d5efa3a0194a66c6ec182baa8f916f8833d#diff-84c9e88bdc298227f2ee856382c6f71544b16819981c51aeca0d987a3cc611bcR385

The Elastic client roadmap for v8.0 calls for dropping the callback style completely in favor of supporting only Promises. https://github.com/elastic/elasticsearch-js/issues/1542 Doing that change removed this particular bug vector https://github.com/elastic/elasticsearch-js/commit/1a227459f096951032b881acce18a01352901096#diff-090a51d7c17b7106cfe4dfedac7909f85c0dbc5b79f0f99943c1a76e6ad5ef45L693-R810

A minimal fix would be to move the destructure of body so that it happens after the code that checks the error and exits with a callback.

[kavilla]

Sorry about the delay. Thank you @apostrophest for the details!

Will try to get some prioritization then!

[curena-contrast]

Bump? Running into this issue with client 2.4.0, Node.js v18.13.0

[dblock]

@curena-contrast Want to pick it up and fix it? At least maybe write a failing test for it?