Skip to content

fix: restore fasterxml JsonParseException mapping in ExceptionsHelper.status() (#21612) [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] - #22632

Open
waterWang wants to merge 1 commit into
opensearch-project:mainfrom
waterWang:fix/opensearch-21612-exceptionshelper-status-jsonparse
Open

fix: restore fasterxml JsonParseException mapping in ExceptionsHelper.status() (#21612) [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT]#22632
waterWang wants to merge 1 commit into
opensearch-project:mainfrom
waterWang:fix/opensearch-21612-exceptionshelper-status-jsonparse

Conversation

@waterWang

Copy link
Copy Markdown

Description

PR #21029 (Jackson 3.x migration) changed ExceptionsHelper.status() to use org.opensearch.tools.jackson.core.JsonParseException in the switch statement, but com.fasterxml.jackson.core.JsonParseException (fasterxml variant) is still on the classpath and used by plugins.

Impact

This is a silent behavioral regression. Any plugin that encounters a fasterxml JsonParseException now gets RestStatus.INTERNAL_SERVER_ERROR (500) instead of BAD_REQUEST (400), causing infinite retries in exception-based retry logic (e.g. opensearch-project/anomaly-detection#1714).

Fix

Added fully-qualified com.fasterxml.jackson.core.JsonParseException cases to both status() and summaryMessage() methods, so both Jackson 2.x (fasterxml) and 3.x (opensearch-tools) variants are handled correctly.

Related Issues

Resolves #21612

….status() (opensearch-project#21612)

PR opensearch-project#21029 (Jackson 3.x migration) changed ExceptionsHelper.status() to use
org.opensearch.tools.jackson.core.JsonParseException in the switch statement,
but com.fasterxml.jackson.core.JsonParseException (fasterxml variant) is still
on the classpath and used by plugins.

This is a silent behavioral regression. Any plugin that encounters a fasterxml
JsonParseException now gets RestStatus.INTERNAL_SERVER_ERROR (500) instead of
BAD_REQUEST (400), causing infinite retries in exception-based retry logic.

Added fully-qualified fasterxml cases to both status() and summaryMessage()
to handle both Jackson 2.x and 3.x variants.

[fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT]
@waterWang
waterWang requested a review from a team as a code owner August 2, 2026 05:18
@github-actions github-actions Bot added the bug Something isn't working label Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid unreachable duplicate pattern case

The added com.fasterxml.jackson.core.JsonParseException case is unreachable because
InputCoercionException (previous case) already extends it, and the existing
JsonParseException case likely refers to the same or a parent class. Verify the
fully-qualified class of the pre-existing JsonParseException import; if it is
already com.fasterxml.jackson.core.JsonParseException, this new case will cause a
compile error for duplicate/unreachable pattern. If it refers to a different type
(e.g., a shaded copy), ensure the ordering places more specific types first.

libs/core/src/main/java/org/opensearch/ExceptionsHelper.java [134]

 case JsonParseException ignored -> RestStatus.BAD_REQUEST;
-case com.fasterxml.jackson.core.JsonParseException ignored -> RestStatus.BAD_REQUEST;
Suggestion importance[1-10]: 7

__

Why: Valid concern: if the existing JsonParseException import already refers to com.fasterxml.jackson.core.JsonParseException, the new case would be a duplicate and cause a compile error. Worth verifying, though the PR author likely added it intentionally for a different (shaded) type.

Medium
Remove potentially duplicate switch case

Same concern in summaryMessage: two switch cases handling JsonParseException
variants may be duplicates or unreachable depending on the type resolution of the
existing import. Confirm that these are distinct types (e.g., a repackaged Jackson
class) and, if not, remove the duplicate case to prevent compile errors and dead
code.

libs/core/src/main/java/org/opensearch/ExceptionsHelper.java [148]

 case JsonParseException ignored -> ErrorMessages.JSON_PARSE_FAILED;
-case com.fasterxml.jackson.core.JsonParseException ignored -> ErrorMessages.JSON_PARSE_FAILED;
Suggestion importance[1-10]: 7

__

Why: Same valid concern applied to summaryMessage; duplicate case could cause compile errors depending on import resolution.

Medium

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 00719d4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 00719d4: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] ExceptionsHelper.status() no longer maps com.fasterxml.jackson.core.JsonParseException to BAD_REQUEST

1 participant