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

Daemon returns different formats of error message #1808

Closed
Nick-S-2018 opened this issue Feb 8, 2024 · 12 comments
Closed

Daemon returns different formats of error message #1808

Nick-S-2018 opened this issue Feb 8, 2024 · 12 comments
Assignees

Comments

@Nick-S-2018
Copy link
Collaborator

In some cases, daemon can return different formats of error message for the same operation. The returned format depends on a specific error. E.g., here are two responses for the insert request when daemon is used without Buddy:

curl -X POST localhost:9308/sql?mode=raw -d "query=create table test(f text)"
[{"total":0,"error":"","warning":""}]

curl -sX POST http://localhost:9308/insert  -d '{
  "index":"test",
  "id":1,
  "doc":
  {
    "f" : "a"               
  }
}'
{"_index":"test","_id":1,"created":true,"result":"created","status":201}

curl -sX POST http://localhost:9408/insert  -d '{
  "index":"test",
  "id":1,
  "doc":
  {
    "f" : "a"
  }
}'
{"error":{"type":"duplicate id '1'","index":"test"},"status":409}

curl -sX POST http://localhost:9408/insert  -d '{
  "index":"test",
  "id":-1,
  "doc":
  {
    "f" : "a"
  }
}'
{"error":"Negative document ids are not allowed"}

This can lead to problems with our OpenAPI-generated clients, so we should think of unifying those response formats in some way.

@tomatolog
Copy link
Contributor

/sql endpoint returns the exact reply from buddy wrapped into array

we could change that but when multi queries will not work - maybe it needs another buddy replication protocol improvement

@Nick-S-2018
Copy link
Collaborator Author

Nick-S-2018 commented Feb 9, 2024

I think, it would be enough to implement a unified format just for insert/replace/update/delete endpoints. As the problem with OpenAPI clients seems to affect only error responses from those ones. So we won't need to change the current handling of /sql or other endpoints.
Can we add the type, index and status properties to all the error responses where they're absent now?
E.g.:
{"error":"Negative document ids are not allowed"}
would become:
{"error":{"type":"Negative document ids are not allowed", "index":"test"},"status":"some appropriate status code"}
and so on.

@sanikolaev
Copy link
Collaborator

As discussed, please provide more info on how the queries are executed (i.e. what Buddy receives/returns)

@sanikolaev
Copy link
Collaborator

It looks like it was assigned to Nick by mistake. Reassigning to @tomatolog to estimate.

@sanikolaev
Copy link
Collaborator

This issue blocks manticoresoftware/openapi#16

@tomatolog
Copy link
Contributor

fixed at the https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error branch as I changed error format for insert/replace/update/delete HTTP JSON endpoints to be the same as it is in the compatibility mode, ie has error object with the string properties type and reason and status. The type has the predefined type of the error from the list searchdhttp.cpp#L3418 and the reason is the error message.

{
    "error": {
        "type": "parse_exception",
        "reason": "Content-Type must be application/x-ndjson"
    },
    "status": 400
}

That is different from the initial description at #1808 (comment) however that reduces the error formats for HTTP JSON endpoints.

I push the change in the branch as think that could break the different APIs and this change should be merged into master along with the API change.

@tomatolog
Copy link
Contributor

I'd also add different types of error.type as now all insert/replace/update/delete returns only parse_exception and runtime error action_request_validation_exception but that is a large change and I'd add it after the branch got merged into master

@tomatolog tomatolog assigned Nick-S-2018 and unassigned tomatolog Apr 4, 2024
@sanikolaev
Copy link
Collaborator

Waiting for #1810 and #1866 to be completed before we can merge this one into the master branch.

@sanikolaev sanikolaev added the waiting Waiting for the original poster (in most cases) or something else label Apr 16, 2024
@sanikolaev
Copy link
Collaborator

Waiting for #1810 and #1866 to be completed before we can merge this one into the master branch.

Unblocked.

@sanikolaev sanikolaev removed the waiting Waiting for the original poster (in most cases) or something else label May 15, 2024
@Nick-S-2018
Copy link
Collaborator Author

I've tested it with the doc_post_error branch - it appears that the response format is always {"error":"Some error message"} without Buddy now.

@Nick-S-2018 Nick-S-2018 assigned tomatolog and unassigned Nick-S-2018 May 20, 2024
@tomatolog
Copy link
Contributor

unable to reproduce - use daemon from the https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error and the requests from the issue description without buddy and all works fine - I see the only new error format

curl -sX POST http://localhost:9408/insert  -d '{
  "index":"test",
  "id":1,
  "doc":
  {
    "f" : "a"
  }
}'
{
  "error": {
    "type": "action_request_validation_exception",
    "reason": "duplicate id '1'",
    "index": "test"
  },
  "status": 409
}
curl -sX POST http://localhost:9408/insert  -d '{
  "index":"test",
  "id":-1,
  "doc":
  {
    "f" : "a"
  }
}'
{
  "error": {
    "type": "parse_exception",
    "reason": "Negative document ids are not allowed",
    "index": "test"
  },
  "status": 400
}

to investigate issue further I need the requests examples along with replies to make sure we check the same case

@tomatolog tomatolog assigned Nick-S-2018 and unassigned tomatolog May 21, 2024
@Nick-S-2018
Copy link
Collaborator Author

Obviously, there was an error with my previous tests. After I've rebuilt Manticore and repeated the tests, the issue is not reproduced. The response formats are returned just as expected without Buddy.

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