[BUG] Unconsumed params error message hides Extension REST request timeouts

[dbwiddis]

What is the bug?

When an extension REST request times out or otherwise fails in transport, the error returned to the user is blocked by unconsumed parameters, hiding the timeout failure.

How can one reproduce the bug?

1. Create a REST action on an extension that times out. (Adding a response delay into a HelloWorld action works.)
2. Send a REST request that contains a parameter.
3. Wait for the request to timeout.
4. Observe the error message about unconsumed params:

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "request [/_extensions/_hw/hello/test] contains unrecognized parameter: [name]"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "request [/_extensions/_hw/hello/test] contains unrecognized parameter: [name]"
  },
  "status": 400
}

What is the expected behavior?

The actual exception causing the failure; in this case a timeout.

Do you have any additional context?

Parameters are normally consumed via param() on the request. In an extension environment, we relay back the consumed parameters as a list and manually param() them again in the onResponse() portion of RestSendToExtensionAction, see here:
https://github.com/opensearch-project/OpenSearch/blob/5f8193021215cd6979fde66474ec8d74d32ac91a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java#L145

However in the case of an exception, this is not done (because we don't have access to the request) and we don't really know the truth about it.

We could probably just consume all the params in the original request anyway.

For background, this is the commit that put this check in, it was intended to provide a "did you mean" helpful result to users who made typos: opensearch-project/OpenSearch@9a83ded

[varuntumbe]

Hi @dbwiddis ,

I would like to take this up next once i merge my current one

Thanks

[dbwiddis]

Great, @varuntumbe ... assigning it to you.

[varuntumbe]

Hi @dbwiddis ,

I will start working on this from tomorrow. I was busy for a couple of days

Thanks

[varuntumbe]

Hi @dbwiddis ,

Apologies for the delay, I was busy last week as well. I started looking at the issue, so far what I have understood is

1. When we send a Rest Request to extension from open search, In extension sdk side we already would have RestHandlers ( for PUT, GET and POST for each extension running in that node ) registered with extensionRestPathRegistry.

2. When open search sends a REST req to extension, depending upon req method and path we get handler function

3. In that handler func we use String name = request.param("name"); here basically we are getting the param name from the req and as a side effect we update consumedParams.add(key);

4. while sending the response we are sending that consumedParam as a part of response.

   public ExtensionRestResponse(RestRequest request, RestStatus status, String content) {
   super(status, content);
   this.consumedParams = request.consumedParams();
   this.contentConsumed = request.isContentConsumed();
   }

5. In our case if we delay the response from extension sdk side, instead of getting the timeout error, we are getting the illegal_argument_exception in opensearch even though we have consumed all param properly.

Can you once confirm whether I am on the right track or not ? I will try to repro this.. this is what I could understand by looking at the code briefly

Thanks and Regards
Varun

[dbwiddis]

5. In our case if we delay the response from extension sdk side, instead of getting the timeout error, we are getting the illegal_argument_exception in opensearch even though we have consumed all param properly.

Can you once confirm whether I am on the right track or not ? I will try to repro this.. this is what I could understand by looking at the code briefly

Correct. Even if we have consumed all the params, in the case of a timeout SDK never gives that information to OpenSearch. There is code to process those consumed params from the extension and "consume" them on the OpenSearch side to match, but in the case of an exception, it doesn't run.

So the fix is in the exception branch, just consume all the params.

[dbwiddis]

Hi @varuntumbe are you still working on this?

[varuntumbe]

Hi @dbwiddis ,

Sorry for the delayed reply. I could not complete this one, I could not spent time on it as i got extremely busy at office work.

Please reassign it to someone else. I will reach out to if I get some free time.

Thanks and Regards
Varun