elastic / elasticsearch-js

Official Elasticsearch client library for Node.js
https://ela.st/js-client
Apache License 2.0
26 stars 728 forks source link

request body is required #1830

Open ronag opened 1 year ago

ronag commented 1 year ago

While calling:

  assert(operations.length)
  await esc.bulk({ operations })

We get the following server error. Looks like an esc client bug? Even

{
  "level": 50,
  "time": 1678970629733,
  "name": "logforwarder",
  "err": {
    "type": "ResponseError",
    "message": "parse_exception\n\tRoot causes:\n\t\tparse_exception: request body is required",
    "stack": "ResponseError: parse_exception\n\tRoot causes:\n\t\tparse_exception: request body is required\n    at SniffingTransport.request (/usr/src/app/node_modules/@elastic/transport/lib/Transport.js:479:27)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async Client.BulkApi [as bulk] (/usr/src/app/node_modules/@elastic/elasticsearch/lib/api/api/bulk.js:51:12)\n    at async file:///usr/src/app/src/index.js:181:41",
    "name": "ResponseError",
    "meta": {
      "body": {
        "error": {
          "root_cause": [
            {
              "type": "parse_exception",
              "reason": "request body is required"
            }
          ],
          "type": "parse_exception",
          "reason": "request body is required"
        },
        "status": 400
      },
      "statusCode": 400,
      "headers": {
        "x-elastic-product": "Elasticsearch",
        "content-type": "application/json;charset=utf-8",
        "content-length": "163"
      },
      "meta": {
        "context": null,
        "request": {
          "params": {
            "method": "POST",
            "path": "/_bulk",
            "body": "",
            "querystring": "",
            "headers": {
              "user-agent": "elastic-transport-js/8.3.1 (linux 5.15.83-0-lts-x64; Node.js v18.13.0)",
              "x-elastic-client-meta": "es=8.6.0,js=18.13.0,t=8.3.1,hc=18.13.0",
              "accept": "application/vnd.elasticsearch+json; compatible-with=8,text/plain"
            }
          },
          "options": {},
          "id": 54354
        },
        "name": "elasticsearch-js",
        "connection": {
          "url": "http://elasticsearch-log:9201/",
          "id": "http://elasticsearch-log:9201/",
          "headers": {},
          "status": "alive"
        },
        "attempts": 0,
        "aborted": false
      },
      "warnings": null
    }
  },
  "msg": "Indexing error"
}

Getting this server response should not even be possible since the client should validate and throw an error even if we the user are doing something wrong...

"@elastic/elasticsearch@^8.5.0":
  version "8.6.0"
  resolved "https://registry.yarnpkg.com/@elastic/elasticsearch/-/elasticsearch-8.6.0.tgz#c474f49808deee64b5bc5b8f938bf78f4468cb94"
  integrity sha512-mN5EbbgSp1rfRmQ/5Hv7jqAK8xhGJxCg7G84xje8hSefE59P+HPPCv/+DgesCUSJdZpwXIo0DwOWHfHvktxxLw==
  dependencies:
    "@elastic/transport" "^8.3.1"
    tslib "^2.4.0"

"@elastic/transport@^8.3.1":
  version "8.3.1"
  resolved "https://registry.yarnpkg.com/@elastic/transport/-/transport-8.3.1.tgz#e7569d7df35b03108ea7aa886113800245faa17f"
  integrity sha512-jv/Yp2VLvv5tSMEOF8iGrtL2YsYHbpf4s+nDsItxUTLFTzuJGpnsB/xBlfsoT2kAYEnWHiSJuqrbRcpXEI/SEQ==
  dependencies:
    debug "^4.3.4"
    hpagent "^1.0.0"
    ms "^2.1.3"
    secure-json-parse "^2.4.0"
    tslib "^2.4.0"
    undici "^5.5.1"
JoshMock commented 1 year ago

Thanks for the report @ronag! In many cases we defer validation to Elasticsearch itself for simplicity's sake, but ideally the client should validate required parameters like body before sending the request, like you're suggesting. I'll try to take a look at this soon.

bijela-gora commented 1 year ago

@ronag hello

You wrote:

the client should validate and throw an error

May I ask what additional value this creates for library users?

JoshMock commented 1 year ago

@bijela-gora The value of client-side validation is that it makes validation much faster by having it happen in-process rather than deferring to a remote server. It also prevents unnecessary network traffic and load on Elasticsearch, and allows for more developer-friendly feedback.

bijela-gora commented 1 year ago

Hi! Thank you for your response.

In my previous post, I referred to specific numbers differences, specifically related to feedback speed. In the context of feedback speed there are three things we can use to improve:

Considering these factors, I believe client-side runtime validation may not offer significant value to end users. Focusing on type system improvements might be more beneficial.

Thanks for reading this.

ArchitGajjar commented 2 months ago

HI @JoshMock - Can I work on this issue ? this would be my first contribution to this project ? Thank you!

JoshMock commented 2 months ago

@ArchitGajjar You definitely can. I'm still not entirely sure there's a better solution than what we're already doing, like @bijela-gora mentioned above, but you're welcome to discuss and propose ideas here or in a draft PR.