Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

request body is required #1830

Open
ronag opened this issue Mar 17, 2023 · 4 comments
Open

request body is required #1830

ronag opened this issue Mar 17, 2023 · 4 comments

Comments

@ronag
Copy link

ronag commented Mar 17, 2023

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
Copy link
Member

JoshMock commented May 1, 2023

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
Copy link

@ronag hello

You wrote:

the client should validate and throw an error

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

@JoshMock
Copy link
Member

JoshMock commented Sep 28, 2023

@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
Copy link

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:

  • Type system: feedback time is code compilation time.
  • Front-end runtime checks: feedback time is compilation time plus execution.
  • Back-end runtime checks: add network delays of up to 300ms on average.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants