Lab 8.3: Improve Error Messages and Diagnostics

Background

When OpenSearch refuses a request, the message it returns is the contract between the engine and the operator staring at a 3 a.m. page. A message like IllegalArgumentException: bad value tells the operator nothing; a message like [size] must be greater than 0. Found [-1] in aggregation [g] tells them the field, the constraint, the offending value, and where it lives. Turning the first into the second is one of the highest-value, lowest-risk contributions you can make — and it is a steady source of real OpenSearch issues.

This lab is a focused contribution type: take a vague exception, validation failure, or log line and make it actionable, then pin the new text with a unit test so it cannot silently regress. The blast radius is small (you change strings and validation, not control flow), but the message becomes part of the contract, so a test must assert it exactly.

Why Good Diagnostics Matter to Operators

• A clear validation message turns a support ticket into a self-service fix.
• Operators grep logs; a message with the field name and value is searchable, a generic one is not.
• The "explain" family of APIs (_cluster/allocation/explain, _validate/query, profile) has no value except the quality of its human-readable text — improving it is pure operator UX.
• Reviewers love these PRs: the change is contained, the test is exact, the benefit is obvious.

Prerequisites

• A built checkout; ./gradlew assemble green.
• Lab 8.2 (the PR mechanics — minimal diff, SPDX, DCO, CHANGELOG, precommit).
• Read Issue roadmap Stage 3: Error Messages and the REST layer deep dive.

Where Diagnostics Live

flowchart LR
  A[HTTP request] --> B[RestHandler.prepareRequest]
  B -->|bad syntax| E1[IllegalArgumentException / XContentParseException]
  B --> C[ActionRequest.validate]
  C -->|null/empty/conflicting fields| E2[ActionRequestValidationException]
  C --> D[TransportAction.doExecute]
  D -->|runtime failure| E3[OpenSearchException subclass]
  E1 --> R[RestController error path -> JSON + RestStatus]
  E2 --> R
  E3 --> R

The four properties of a good message — say them as a checklist before you write any string:

1. What failed (the parameter/field name, in brackets: [size]).
2. Why (the constraint: must be greater than 0).
3. The offending value (Found [-1]).
4. Where / what to do (location, or the valid set/range).

Step-by-Step Tasks

Step 1 — Find a vague message (15 min)

Hunt for low-quality diagnostics. Generic exceptions with no context are the target:

# IllegalArgumentExceptions with a bare, context-free message
grep -rn 'IllegalArgumentException("' server/src/main/java/org/opensearch/ \
  | grep -viE '\[|\{|" \+ |must|expected|invalid|unknown|found' | head -20

# validate() methods that addValidationError with vague text
grep -rn "addValidationError" server/src/main/java/org/opensearch/ | head -20

# log lines that fire with no identifying context
grep -rn 'logger\.\(warn\|error\)("' server/src/main/java/org/opensearch/ \
  | grep -viE '\{|" \+ ' | head -20

Pick one where you can show that a user could hit it with a bad request, and where the current text is unhelpful. For this lab the worked example is a request validation improvement.

Step 2 — Reproduce the bad message (10 min)

Start a node and provoke it so you can quote the current (bad) output:

./gradlew run
export OS=http://localhost:9200

# Example: an _mtermvectors / bulk / reindex request missing a required field,
# or a setting update with an out-of-range value. Provoke and capture:
curl -s -i -XPUT "$OS/x/_settings" -H 'Content-Type: application/json' -d '{
  "index": { "number_of_replicas": -3 } }' | sed -n '1,15p'

Record the status and the error.reason. If the message already names the field and value, find a worse one — you want a genuine improvement.

Step 3 — Locate the message in source (15 min)

Grep the exact (or near-exact) text you saw:

grep -rn "number_of_replicas\|must be" \
  server/src/main/java/org/opensearch/cluster/metadata/ \
  server/src/main/java/org/opensearch/common/settings/ | head

Read the throwing site. Decide which surface it belongs to (validation vs parse vs runtime) using the table above. Do not invent a line number — read the code the grep points at.

Step 4 — Improve the message (15 min)

Apply the four-property checklist. A request-validation example — turning a context-free error into a precise one:

--- a/server/src/main/java/org/opensearch/action/admin/indices/open/OpenIndexRequest.java
+++ b/server/src/main/java/org/opensearch/action/admin/indices/open/OpenIndexRequest.java
@@ public ActionRequestValidationException validate() {
     ActionRequestValidationException validationException = null;
     if (CollectionUtils.isEmpty(indices)) {
-        validationException = addValidationError("index is missing", validationException);
+        validationException = addValidationError(
+            "index is missing; specify one or more index names or patterns (e.g. \"my-index\" or \"logs-*\")",
+            validationException
+        );
     }
     return validationException;
 }

An argument-error example — naming the parameter, value, and valid range:

--- a/server/src/main/java/org/opensearch/.../SomeBuilder.java
+++ b/server/src/main/java/org/opensearch/.../SomeBuilder.java
@@
-    if (windowSize < 1) {
-        throw new IllegalArgumentException("invalid window size");
-    }
+    if (windowSize < 1) {
+        throw new IllegalArgumentException(
+            "[window_size] must be at least 1. Found [" + windowSize + "]"
+        );
+    }

Note: Keep the message machine-stable enough to test but human-first. Put field names in [brackets] (the OpenSearch convention) so they are greppable. Do not leak internal class names or stack frames into operator-facing text — that is for logs, not the REST error body.

Step 5 — Pin the message with a unit test (20 min)

A message is now part of the contract; a test must assert it so a future refactor cannot silently regress it. For validate():

/*
 * SPDX-License-Identifier: Apache-2.0
 *
 * The OpenSearch Contributors require contributions made to
 * this file be licensed under the Apache-2.0 license or a
 * compatible open source license.
 */
package org.opensearch.action.admin.indices.open;

import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.test.OpenSearchTestCase;

public class OpenIndexRequestValidationTests extends OpenSearchTestCase {

    public void testMissingIndexGivesActionableMessage() {
        OpenIndexRequest request = new OpenIndexRequest(); // no indices set
        ActionRequestValidationException e = request.validate();
        assertNotNull("validate() should reject a request with no indices", e);
        assertThat(e.getMessage(), containsString("index is missing"));
        // The actionable part — the part operators actually need:
        assertThat(e.getMessage(), containsString("specify one or more index names or patterns"));
    }
}

For an IllegalArgumentException from a builder, assert with expectThrows:

public void testNegativeWindowSizeMessage() {
    IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SomeBuilder().windowSize(-1));
    assertThat(e.getMessage(), containsString("[window_size] must be at least 1"));
    assertThat(e.getMessage(), containsString("Found [-1]"));
}

Run it, and verify it is red on main (the old message lacked the new text) and green with your diff:

./gradlew :server:test --tests "*.OpenIndexRequestValidationTests"

Warning: Assert the actionable substring, not the entire message string. Pinning the exact full string makes the test brittle to harmless rewording and invites reviewers to ask you to relax it. Pin the load-bearing words (field name, value, constraint).

Step 6 — (Optional) verify the REST rendering and status (10 min)

If the message surfaces over REST, confirm the status code and error body with a REST-YAML test — operators see the JSON, not the Java exception:

---
"open index with no name returns an actionable 400":
  - do:
      catch: /index is missing/
      indices.open:
        index: ""
  - match: { status: 400 }

grep -rn "RestStatus" server/src/main/java/org/opensearch/action/admin/indices/open/ | head
./gradlew :rest-api-spec:yamlRestTest --tests "*open*" 2>/dev/null || true

Confirm the exception maps to RestStatus.BAD_REQUEST (400), not a 500 — a validation problem is the client's fault, and the status must say so. If it renders as 500, that itself is a worthwhile fix.

Step 7 — Ship it as a PR (10 min)

Follow Lab 8.2 exactly: minimal diff, SPDX headers, spotlessApply, CHANGELOG.md entry, DCO sign-off, precommit, and the PR template.

./gradlew spotlessApply :server:precommit
git add -A
git commit -s -m "Improve OpenIndexRequest validation message

Make the 'index is missing' validation error actionable by telling the
operator to supply index names or patterns. Pins the message with a test.

Closes #NNNNN"

CHANGELOG line:

 ### Changed
+- Make `OpenIndexRequest` validation error actionable when no index is supplied ([#NNNNN](https://github.com/opensearch-project/OpenSearch/pull/NNNNN))

Note: Diagnostic-only changes are usually Changed (not Fixed) unless the old message was outright wrong/misleading. Match the category to the nature of the change.

Implementation Requirements

• A located vague message with the current (bad) output captured.
• A rewritten message satisfying the four-property checklist (what / why / value / where).
• A unit test asserting the actionable substring(s) — red on main, green with the fix.
• (If REST-surfaced) confirmation the status is the correct 4xx, optionally a REST-YAML test.
• A merge-quality PR: minimal diff, SPDX, CHANGELOG, DCO sign-off, precommit green.

Expected Output

OpenIndexRequestValidationTests > testMissingIndexGivesActionableMessage PASSED
BUILD SUCCESSFUL

And, against a running node, an error body that now reads e.g.:

{ "error": { "type": "action_request_validation_exception",
  "reason": "Validation Failed: 1: index is missing; specify one or more index names or patterns (e.g. \"my-index\" or \"logs-*\");" },
  "status": 400 }

Troubleshooting

Stretch Goals

1. Improve a message in the _cluster/allocation/explain path — find where an unassigned shard's reason text is built and make it name the exact decider that blocked allocation:

   grep -rn "AllocationDecision\|explain\|NO\b" server/src/main/java/org/opensearch/cluster/routing/allocation/ | grep -i explain | head
   

2. Convert a multi-problem validate() to emit one entry per problem (so an operator fixing a request sees all the issues at once, not one-at-a-time). Test that two problems produce two entries.
3. Find a logger.warn/error that fires without identifying context (no index/shard/node id) and add it, with a test or a documented manual grep of the log proving the new format.

Validation / Self-check

1. Quote the message before and after. Which of the four properties did it gain?
2. Does your test pin the actionable substring, not the whole string? Why does that matter?
3. Is the message red on main and green with your fix? Show it.
4. If it surfaces over REST, is the status a correct 4xx (client error), not a 500?
5. Did you avoid leaking internal class/stack detail into operator-facing text?
6. Why are diagnostics improvements valued by both operators and reviewers?

Cross-references: Issue roadmap Stage 3: Error Messages, REST layer deep dive, Lab 8.2: Implement the Fix, Action Framework.