Skip to content

fix: Return 400 for non-integer min_gram/max_gram values - #23035

Open
vaibhoag wants to merge 1 commit into
opensearch-project:mainfrom
vaibhoag:fix-gram-errorcode
Open

vaibhoag wants to merge 1 commit into
opensearch-project:mainfrom
vaibhoag:fix-gram-errorcode

Conversation

@vaibhoag

Copy link
Copy Markdown
Contributor

Description

Returns HTTP 400 (illegal_argument_exception) instead of HTTP 500 when a non-integer value is supplied for min_gram / max_gram in the ngram and edge_ngram tokenizers and token filters.

Gram sizes are character counts and must be integers. Today, a value such as 1.0 — commonly produced by client-side JSON serializers that emit whole numbers as floats — is stored verbatim in Settings and fails in Settings#getAsInt with a SettingsException, which surfaces to the caller as an internal server error. That masks the actionable message and encourages clients to retry a request that can never succeed:

PUT /my-index
{
  "settings": {
    "analysis": {
      "tokenizer": {
        "my_ngram": { "type": "ngram", "min_gram": 1.0, "max_gram": 2 }
      }
    }
  }
}

Before: 500OpenSearch exception [type=settings_exception, reason=Failed to parse int setting [min_gram] with value [1.0]]

After: 400illegal_argument_exception: [min_gram] must be an integer, got [1.0]

Changes:

  • Adds Settings#getAsIntStrict(String, Integer): identical to getAsInt for absent and valid values, but throws IllegalArgumentException (HTTP 400) naming the setting and the offending value when the value cannot be parsed as an integer. Intended for reads of caller-supplied values where the setting is defined to be integral.
  • Uses it for the min_gram/max_gram reads in NGramTokenizerFactory, EdgeNGramTokenizerFactory, NGramTokenFilterFactory, and EdgeNGramTokenFilterFactory. Analysis components are built during index creation, so a malformed gram size now fails CreateIndex fast with a clear client error.
  • Settings#getAsInt and SettingsException semantics are unchanged for all other callers; settings parse failures outside caller-supplied input keep their existing behavior.

Testing:

  • SettingsTests#testGetAsIntStrict — absent/valid/unparseable values, and asserts the lenient getAsInt still throws SettingsException for the same value.
  • NGramTokenizerFactoryTests#testNonIntegerGramSizeIsClientError — end-to-end through the real tokenizer factory: asserts the exact message and the NumberFormatException cause.

Related Issues

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Vaibhav Agarwal <vaibhoag@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 01b24ed: SUCCESS

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.75%. Comparing base (f4918fa) to head (01b24ed).

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #23035      +/-   ##
============================================
+ Coverage     71.73%   71.75%   +0.01%     
+ Complexity    77670    77621      -49     
============================================
  Files          6168     6168              
  Lines        360106   360112       +6     
  Branches      52379    52380       +1     
============================================
+ Hits         258314   258390      +76     
+ Misses        81254    81164      -90     
- Partials      20538    20558      +20     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vaibhoag
vaibhoag marked this pull request as ready for review September 15, 2026 08:16
@vaibhoag
vaibhoag requested a review from a team as a code owner September 15, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant