manticoresearch
manticoresearch copied to clipboard
Daemon does not pass to Buddy all information about error.
In some cases, when error data is a complex object that contains multiple properties, daemon does not pass them all to Buddy.
create table test(f text);
curl -sX POST http://localhost:9308/insert -d '{
"index":"test",
"id":1,
"doc":
{
"f" : "test",
}
}'
{"_index":"test","_id":1,"created":true,"result":"created","status":201}
curl -sX POST http://localhost:9308/insert -d '{
"index":"test",
"id":1,
"doc":
{
"f" : "test",
}
}'
In this case, the response without Buddy is:
{"error":{"type":"duplicate id '1'","index":"test"},"status":409}
but with Buddy is:
[{"total":0,"warning":"","error":"duplicate id '1'"}]
We need to pass all data related to the error to Buddy, otherwise it cannot be returned to the client.
Blocked by https://github.com/manticoresoftware/manticoresearch/issues/1806
Once https://github.com/manticoresoftware/manticoresearch/issues/1806 is done, @Nick-S-2018 pls check if this issue is still actual
Blocked by https://github.com/manticoresoftware/manticoresearch/issues/1806
Unblocked as it's done.
It's fixed in the in the buddy_http_code
branch. We'll merge that into master once we finish the other issues related to the future update of our OpenAPI YAML schema.
@Nick-S-2018 is this issue closed by mistake?
We'll merge that into master once we finish the other issues related to the future update of our OpenAPI YAML schema.
Should it be blocked by another one?
Yes, it's been blocked by https://github.com/manticoresoftware/manticoresearch/issues/1808 as well. I've reopened it now.
We need daemon to pass extra info about error to Buddy, similarly to how daemon passes it to user without Buddy.
I.e., in the following example, info from type
, index
(if exists) , and status
fields needs to be passed to Buddy as parts of the request as well.
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
}
I checked daemons from the master version and the head of the buddy_http_code
branch with buddy running and without and all the time for the requests with the body
{
"index":"test",
"id":-1,
"doc":
{
"f" : "a"
}
}
I alway see the simple error without any objects and so one
C:\dev\sphinx\build\m_dbg22\src\Debug\txt>curl -sX POST "localhost:9312/insert" -H "content-type: application/json" --data-binary @ins-1807.js | jq
{
"error": "Negative document ids are not allowed"
}
that is why it is not clear what should I pass to buddy. I do not get any of the type
, index
(if exists) , and status
fields
I forgot to mention the tests should be made with the https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error branch
I use daemon from the https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error but with master version of buddy daemon failed to start with the error message
WARNING: [BUDDY] invalid output, should be 'Buddy ver, started address:port', got 'Error while initialization: Call to protected method Manticoresearch\Buddy\Core\Plugin\Pluggable::registerHooks() from scope Manticoresearch\Buddy\Base\Lib\QueryProcessor
still not clear what buddy version should I use for this ticket
I've tested the branch version with these versions from branch's deps.txt
- try to test using them:
buddy 2.3.1 24032906 541c941
executor 1.1.1 24030109 3a4d6c4
pushed into https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error branch the fix https://github.com/manticoresoftware/manticoresearch/commit/371e324d714082c3785a9b0076b2731f5b8c0a2d that we discussed with @Nick-S-2018 .
Now daemon adds new error_body
property to the request to buddy there the whole HTTP body of the reply that daemon sends to client. The buddy can parse and extract all needed fields from the error_body
. As collect separate type
\ index
\ status
types for every HTTP handlers need the large code refactoring.
I've tried creating a PR to build the packages, so it's easier to test, but there are conflicts with the master branch - https://github.com/manticoresoftware/manticoresearch/pull/2317 They perhaps need to be resolved.
I suggest implementing a change in the protocol. Instead of creating another field that may or may not be used, we should consider utilizing the error
field.
I propose using the following structure to make it simpler:
"error": {
"message": "string value with error",
"body": "JSON-encoded extra value" or null
}
I'm not sure if "body" is the best name for what we're writing there. We may also consider using "info" or "details" instead. Suggestions are welcome.
Once we agree and proceed with this, we should also update our protocol here: https://github.com/manticoresoftware/manticoresearch-buddy?tab=readme-ov-file#communication-protocol-v2
On the Buddy side, we should improve the implementation and try to encapsulate the Error into a struct to avoid if-else statements. This way, we'll simply be able to get the message or bodyStruct and do what we need. Currently, in the pull request, we use if-else statements in the error handling section, which may be hard to read and maintain in the future.
@Nick-S-2018 @tomatolog @sanikolaev What your thoughts?
at the dev call all agree this change is the best we could try
pushed into https://github.com/manticoresoftware/manticoresearch/tree/doc_post_error branch the fix https://github.com/manticoresoftware/manticoresearch/commit/7c73ff6162d2322d945ecf23098bcadc4edd6deb with the change that request to buddy has error
object with the message
error string and the body
object or null that is the whole reply of the daemon, ie now the request to buddy looks like
{
"type":"unknown json request",
"error":{
"message":"\"index\" property value should be a string",
"body":{
"error":"\"index\" property value should be a string"
}
},
"version":2,
"message":{
"path_query":"/search",
"body":"{ \"index\" : {\"_index\":\"test\"} }{\"@timestamp\": \"1981-11-16\"}",
"http_method":"POST"
}
}
as discussed here https://github.com/manticoresoftware/manticoresearch/issues/1807#issuecomment-2316814597
@tomatolog
"version":2,
Shouldn't it be version 3?
yes That also should be fixed to ver 3 along with documenting the change
Implemented in Buddy in https://github.com/manticoresoftware/manticoresearch-buddy/pull/341 @donhardman please, review it
I left my comments