[BUG] ShardFailure deserialization fails when OpenSearch omits the primary field in error responses

[janhoy]

What is the bug?

This is a regression in version 3.5.0, introduced by this commit, after fetching latest api-spec and doing code-gen.

In opensearch-java v3.4.0 this worked, but from v3.5.0, the client throws a MissingRequiredPropertyException when attempting to deserialize error responses that contain shard failures, since primary field is now required in the updated api-spec. This occurs during search requests when OpenSearch returns shard-level failures in the error response.

Error:
org.opensearch.client.util.MissingRequiredPropertyException: Missing required property 'ShardFailure.primary'

How can one reproduce the bug?

1. Execute a search query that triggers a shard failure (e.g., malformed query, index with shard issues)
2. OpenSearch returns an error response containing failed_shards in the error metadata
3. The Java client attempts to deserialize the ShardFailure objects
4. Deserialization fails because the primary field is marked as required but OpenSearch omits it

Example error response structure:

{
  "error": {
    "type": "search_phase_execution_exception",
    "reason": "all shards failed",
    "failed_shards": [
      {
        "shard": 0,
        "index": "my-index",
        "node": "node-id",
        "reason": {
          "type": "query_shard_exception",
          "reason": "Parse failure"
        }
      }
    ]
  }
}

Note: The primary field is missing from the failed_shards objects.

What is the expected behavior?

The client should successfully deserialize error responses containing shard failures, regardless of whether the primary field is present or not. The primary field should be treated as optional since OpenSearch doesn't consistently include it in all error responses.

What is your host/environment?

• OpenSearch Java Client version: 3.5

Do you have any screenshots?

No

Do you have any additional context?

Root Cause:
The ShardFailure class (generated from the OpenAPI spec) marks the primary field as required and uses a primitive boolean type, which cannot be null. However, OpenSearch's actual error responses often omit this field, causing a mismatch between the spec and real-world behavior.

Affected Code:

• java-client/src/generated/java/org/opensearch/client/opensearch/_types/ShardFailure.java

Proposed Fix:

• 3.x: Make the primary attribute not required
• 4.x: Introduce ShardSearchFailure schema class distinct from ShardFailure

[reta]

Thank you @janhoy , the fix should go to https://github.com/opensearch-project/opensearch-api-specification/ so all clients would able to benefit from it.

[janhoy]

Thanks for the review @reta. I tried to fix it in the spec, see linked PR. I suppose if/when that gets accepted and a new version of the spec if published, that will automatically fix the bug in next opensearch-java as well. I don't 100% understand the relationship and update cadence of the two. Do you require more from me?

[reta]

Thanks for the review @reta. I tried to fix it in the spec, see linked PR. I suppose if/when that gets accepted and a new version of the spec if published, that will automatically fix the bug in next opensearch-java as well. I don't 100% understand the relationship and update cadence of the two. Do you require more from me?

Thank you @janhoy , yes, once the spec is updated, it will be picked up (one a week scheduled workflow) and regenerate API models, nothing else is required, thank you

[janhoy]

Thank you @janhoy , yes, once the spec is updated, it will be picked up (one a week scheduled workflow) and regenerate API models, nothing else is required, thank you

Please see my comment in api-spec repo. I believe the particular API-spec change that fixes this one bug needs to be back-ported and brought into 3.x branch of this repo and re-generate code-gen to get ready for the 3.7.0 release. There are breaking changes in the latest 4.x api-spec that we cannot take in to 3.x. Also, users of 3.x may already be using code-generated ShardFailure class in their client code, which was probably working for parsing shard-failure response until primary was made a required field. Question is thus, if the 3.x fix should be different from the 4.x fix, which changes the class used. The fix for 3.x could be what I had in my PR first, just making primary not required.

Important The bug is a regression introduced in v3.5.0. A simple workaround is to downgrade to v3.4.0. It should also be checked whether other api-spec changes introduced in that same commit in 3.5.0 may have caused other back-compat breaking changes that are still not discovered.

Please advise.

[reta]

Please see my comment in api-spec repo. I believe the particular API-spec change that fixes this one bug needs to be back-ported and brought into 3.x branch of this repo and re-generate code-gen to get ready for the 3.7.0 release. There are breaking changes in the latest 4.x api-spec that we cannot take in to 3.x

Thanks @janhoy , yes, we try to not backport breaking changes from the spec to 3.x (tedious job), so 4.x and 3.x do have divergence here.

Question is thus, if the 3.x fix should be different from the 4.x fix, which changes the class used. The fix for 3.x could be what I had in my PR first, just making primary not required.

This even less sustainable path to keep two branches in sync, let me see what we can do here, I will bring latest spec to main branch first. Most important thing here - we should never modify generate code.

[reta]

@janhoy so here is the thing, the API for shard failure indeed does not include primary as well as status (snippet below):

 {
      "shard": 0,
      "index": "<index_name>",
      "node": "<node_id>",
      "reason": {
        "type": "node_not_connected_exception",
        "reason": "[<node_name>][<ip>:<port>] Node not connected"
      }
    },

The proper fix would be to remove primary and status from the shard failure (since those would never be populated and would always be null). To preserve backward compatibility I believe we could:

• revert the spec change (not only optionality of it, it is going to be nightmare to keep track of) altogether
• absorb introduction of new ShardSearchFailure - yes, it is breaking, the data model is not right anyway

I would honestly believe introducing ShardSearchFailure would be a better option here. Alternatively, which could be a larger change in spec first, we could revert ShardFailure changes and introduce new class (with primary + status) instead. Lastly, reverting the ShardFailure changes would also work but we won't be able to expose primary properly (adhoc spec changes are off the table to be fair). Thank you

[janhoy]

I trust your fix will be sufficient, will test it in v3.7.0. You may close this issue as I won't have cycles to test a snapshot version right now.