Skip to content

[Bug-fix] Translate JSONException to 400 instead of 500#3833

Merged
Swiddis merged 7 commits into
opensearch-project:mainfrom
RyanL1997:json-400
Jul 7, 2025
Merged

[Bug-fix] Translate JSONException to 400 instead of 500#3833
Swiddis merged 7 commits into
opensearch-project:mainfrom
RyanL1997:json-400

Conversation

@RyanL1997

@RyanL1997 RyanL1997 commented Jun 30, 2025

Copy link
Copy Markdown
Collaborator

Description

Translate JSONException to 400 instead of 500

Related Issues

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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: Jialiang Liang <jiallian@amazon.com>
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
@RyanL1997 RyanL1997 added the bug Something isn't working label Jul 1, 2025
RyanL1997 added 2 commits July 1, 2025 15:50
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Comment thread legacy/src/test/java/org/opensearch/sql/legacy/plugin/RestSQLActionTest.java Outdated

@dai-chen dai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meanwhile do you have error stacktrace? I'm thinking another option is to figure out why this JSON exception is not captured.

@RyanL1997

Copy link
Copy Markdown
Collaborator Author

@dai-chen

Meanwhile do you have error stacktrace? I'm thinking another option is to figure out why this JSON exception is not captured.

Here we go:

[2025-07-02T14:54:39,361][ERROR][o.o.s.l.p.RestSqlAction  ] [xxxxxxxxxxx.ant.amazon.com] ce4ecfbe-a386-4799-a482-2cce38b75e76 Server side error during query execution
org.json.JSONException: JSONObject["query"] is not a string (class org.json.JSONArray).
        at org.json.JSONObject.wrongValueFormatException(JSONObject.java:2906) ~[?:?]
        at org.json.JSONObject.getString(JSONObject.java:859) ~[?:?]
        at org.opensearch.sql.legacy.request.SqlRequestFactory.parseSqlRequestFromPayload(SqlRequestFactory.java:59) ~[?:?]
        at org.opensearch.sql.legacy.request.SqlRequestFactory.getSqlRequest(SqlRequestFactory.java:30) ~[?:?]
        at org.opensearch.sql.legacy.plugin.RestSqlAction.prepareRequest(RestSqlAction.java:119) ~[?:?]
        at org.opensearch.rest.BaseRestHandler.handleRequest(BaseRestHandler.java:113) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.security.filter.SecurityRestFilter$AuthczRestHandler.handleRequest(SecurityRestFilter.java:193) ~[?:?]
        at org.opensearch.rest.RestController.dispatchRequest(RestController.java:381) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.rest.RestController.tryAllHandlers(RestController.java:467) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.rest.RestController.dispatchRequest(RestController.java:287) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.security.ssl.http.netty.ValidatingDispatcher.dispatchRequest(ValidatingDispatcher.java:69) ~[?:?]
        at org.opensearch.http.AbstractHttpServerTransport.dispatchRequest(AbstractHttpServerTransport.java:374) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.http.AbstractHttpServerTransport.handleIncomingRequest(AbstractHttpServerTransport.java:482) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.http.AbstractHttpServerTransport.incomingRequest(AbstractHttpServerTransport.java:357) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.http.netty4.Netty4HttpRequestHandler.channelRead0(Netty4HttpRequestHandler.java:56) ~[?:?]
        at org.opensearch.http.netty4.Netty4HttpRequestHandler.channelRead0(Netty4HttpRequestHandler.java:42) ~[?:?]
        at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:99) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at org.opensearch.http.netty4.Netty4HttpPipeliningHandler.channelRead(Netty4HttpPipeliningHandler.java:72) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:107) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:107) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:107) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:102) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346) ~[?:?]
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:289) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:107) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1519) ~[?:?]
        at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1377) ~[?:?]
        at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1428) ~[?:?]
        at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:530) ~[?:?]
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:469) ~[?:?]
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[?:?]
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) ~[?:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[?:?]
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868) ~[?:?]
        at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166) ~[?:?]
        at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:796) ~[?:?]
        at io.netty.channel.nio.NioEventLoop.processSelectedKeysPlain(NioEventLoop.java:697) ~[?:?]
        at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:660) ~[?:?]
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562) ~[?:?]
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998) ~[?:?]
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[?:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Swiddis
Swiddis previously approved these changes Jul 2, 2025
@dai-chen

dai-chen commented Jul 2, 2025

Copy link
Copy Markdown
Collaborator

@dai-chen

Meanwhile do you have error stacktrace? I'm thinking another option is to figure out why this JSON exception is not captured.

Here we go:

[2025-07-02T14:54:39,361][ERROR][o.o.s.l.p.RestSqlAction ] [xxxxxxxxxxx.ant.amazon.com] ce4ecfbe-a386-4799-a482-2cce38b75e76 Server side error during query execution
org.json.JSONException: JSONObject["query"] is not a string (class org.json.JSONArray).
at org.json.JSONObject.wrongValueFormatException(JSONObject.java:2906) ~[?:?]
at org.json.JSONObject.getString(JSONObject.java:859) ~[?:?]
at org.opensearch.sql.legacy.request.SqlRequestFactory.parseSqlRequestFromPayload(SqlRequestFactory.java:59) ~[?:?]
at org.opensearch.sql.legacy.request.SqlRequestFactory.getSqlRequest(SqlRequestFactory.java:30) ~[?:?]
at org.opensearch.sql.legacy.plugin.RestSqlAction.prepareRequest(RestSqlAction.java:119) ~[?:?]
at org.opensearch.rest.BaseRestHandler.handleRequest(BaseRestHandler.java:113) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]

Got it. In this case, we can capture it in parse payload method for SQL and PPL separately. The reason is this org.json.JSONException is very specific, I think we'd better keep exception list in isClientError clean. Thanks!

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Signed-off-by: Jialiang Liang <jiallian@amazon.com>

@dai-chen dai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@Swiddis Swiddis merged commit c07ed2a into opensearch-project:main Jul 7, 2025
25 checks passed

@noCharger noCharger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RyanL1997 should this backport to 2.x and 2.19-dev?

@RyanL1997

Copy link
Copy Markdown
Collaborator Author

I don't think this one can fall into either the criteria of maintenance and critical bugfix. We can if we consider to include this into the future patch of 2.19.x.

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

Labels

bug Something isn't working v3.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] SQL Plugin returns 500 instead of 400 for JSONException

4 participants