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

[BUG] Endpoint option client.ignore not working anymore with 2.4.x #296

Closed
mfn opened this issue Feb 12, 2025 · 5 comments
Closed

[BUG] Endpoint option client.ignore not working anymore with 2.4.x #296

mfn opened this issue Feb 12, 2025 · 5 comments
Labels
bug Something isn't working untriaged

Comments

@mfn
Copy link

mfn commented Feb 12, 2025

What is the bug?

After upgrading to 2.4.1 from 2.3.x, the option to ignore certain HTTP status codes is not working anymore.

How can one reproduce the bug?

Try getting an alias for an index, but the index does not exist.

$client = \OpenSearch\ClientBuilder::create()->setHosts(['opensearch:9200'])->build();

$client->indices()->getAlias(['name' => 'i_dont_exist', 'client' => ['ignore' => 404]]);

The outcome is that an exception is thrown:

   OpenSearch\Common\Exceptions\Missing404Exception  {"error":"alias [i_dont_exist] missing","status":404}.

ps: I'm aware that ClientBuilder is deprecated

What is the expected behavior?

No exception should be thrown, because I instructed the call to ignore 404, which means a plain array should be returned containing this:

= [
    "error" => "alias [i_dont_exist] missing",
    "status" => 404,
  ]
@mfn mfn added bug Something isn't working untriaged labels Feb 12, 2025
@mfn
Copy link
Author

mfn commented Feb 12, 2025

The problem is that \OpenSearch\TransportInterface::sendRequest as introduced in https://github.com/opensearch-project/opensearch-php/pull/233/files#diff-bca1c9c9af8a560484df9795336e323443a882fd8c07bdeda7208b37a314aaeaR19 misses the array $options argument but \OpenSearch\Client::performRequest passes the $options as $headers:

Image

The interface is coming from https://github.com/opensearch-project/opensearch-php/pull/233/files#diff-bca1c9c9af8a560484df9795336e323443a882fd8c07bdeda7208b37a314aaeaR19 and there was a realization that the previous exception behaviour need to be restored with #245

The last PR added the $endpoint->getOptions(); in https://github.com/opensearch-project/opensearch-php/pull/245/files#diff-2d80026e440d8c7862c5a356f3b962d849d9f5a6e729073637a14ee24068aae0R2140 but failed to account that this is are the $headers and the interface lacks the $options.

But this means there's more missing, because that $options would need to be passed through to further methods, which all lack this.

I guess that's why in \OpenSearch\Connections\Connection::process4xxError, which is responsible for handling the ignore part, the actual $ignore array always ends up empty.


I hope this analysis helps a bit and I wish I could make a PR to fix this, but I don't know this project well and the all around impact is way over my head, sorry.

@shyim
Copy link
Collaborator

shyim commented Feb 12, 2025

sure, make a PR if you have already debugged that so far. A test would be nice if you don't feel familiar with that, it is also fine. I would add later a test

@mfn
Copy link
Author

mfn commented Feb 12, 2025

As I said, this is over my head: I see the $options is not just missing in the interface but the implementations using that interface so no idea how to approach this, sorry.

@kimpepper
Copy link
Collaborator

This is fixed in #300

@mfn
Copy link
Author

mfn commented Feb 27, 2025

@kimpepper I upgraded to 2.4.2 but my provided code snippet still fails: throws an exception, when it shouldn't?

Try getting an alias for an index, but the index does not exist.

$client = \OpenSearch\ClientBuilder::create()->setHosts(['opensearch:9200'])->build();

$client->indices()->getAlias(['name' => 'i_dont_exist', 'client' => ['ignore' => 404]]);

The outcome is that an exception is thrown:

   OpenSearch\Common\Exceptions\Missing404Exception  {"error":"alias [i_dont_exist] missing","status":404}.

But it shouldn't, the ignore should be passed down and ultimately the exception should not be thrown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged
Projects
None yet
Development

No branches or pull requests

3 participants