This is an automated email from the ASF dual-hosted git repository.

zhengruifeng pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.x by this push:
     new 9992c82566d3 [SPARK-56926][CONNECT] Guard against null 
messageParameters in Connect error proto serialization
9992c82566d3 is described below

commit 9992c82566d3cc76509efa00c8b2b7d361488cca
Author: haoyangeng-db <[email protected]>
AuthorDate: Wed May 20 09:03:18 2026 +0800

    [SPARK-56926][CONNECT] Guard against null messageParameters in Connect 
error proto serialization
    
    ### What changes were proposed in this pull request?
    
    Null-guard `sparkThrowableBuilder.putAllMessageParameters` in 
`ErrorUtils.throwableToProtoErrors` (Spark Connect server). If a 
`SparkThrowable` implementation returns `null` from `getMessageParameters()`, 
the unguarded call to `putAllMessageParameters(null)` throws a 
`NullPointerException`. With the guard, a `null` map is treated the same as an 
empty one.
    
    ### Why are the changes needed?
    
    The `SparkThrowable` interface's default `getMessageParameters` returns an 
empty map, but an override may violate that implicit non-null contract. When it 
does, the resulting NPE inside `FetchErrorDetails` server-side serialization 
crashes the RPC, and the client falls back to a degraded error reconstruction 
that drops the cause chain, hiding the underlying error from the user.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No. Well-behaved `SparkThrowable` implementations are unaffected. A faulty 
override that returns `null` now serializes with an empty `messageParameters` 
map instead of crashing the RPC.
    
    ### How was this patch tested?
    
    Added a regression test in `FetchErrorDetailsHandlerSuite` that uses an 
inline `SparkThrowable` whose `getMessageParameters` returns `null` and 
verifies the response is built successfully with an empty `messageParameters` 
map.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Drafted with Claude Code
    
    Closes #55992 from haoyangeng-db/spark-connect-guard-null-message-params.
    
    Authored-by: haoyangeng-db <[email protected]>
    Signed-off-by: Ruifeng Zheng <[email protected]>
    (cherry picked from commit a93ffcfd63747388b18ccefa4ea66d61daac445e)
    Signed-off-by: Ruifeng Zheng <[email protected]>
---
 .../apache/spark/sql/connect/utils/ErrorUtils.scala   |  8 +++++++-
 .../service/FetchErrorDetailsHandlerSuite.scala       | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git 
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
 
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
index 619fefe2e7c6..588493aeaa92 100644
--- 
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
+++ 
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
@@ -161,7 +161,13 @@ private[connect] object ErrorUtils extends Logging {
             
breakingChangeInfoBuilder.setNeedsAudit(breakingChangeInfo.needsAudit)
             
sparkThrowableBuilder.setBreakingChangeInfo(breakingChangeInfoBuilder.build())
           }
-          
sparkThrowableBuilder.putAllMessageParameters(sparkThrowable.getMessageParameters)
+          // The SparkThrowable interface's default getMessageParameters 
returns an empty
+          // map, but a faulty override may return null. Guard against that so 
the
+          // FetchErrorDetails RPC doesn't crash with a NullPointerException.
+          val messageParameters = sparkThrowable.getMessageParameters
+          if (messageParameters != null) {
+            sparkThrowableBuilder.putAllMessageParameters(messageParameters)
+          }
           builder.setSparkThrowable(sparkThrowableBuilder.build())
         case _ =>
       }
diff --git 
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
 
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
index c74dea3418f0..813a57b88936 100644
--- 
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
+++ 
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
@@ -369,4 +369,23 @@ class FetchErrorDetailsHandlerSuite extends 
SharedSparkSession with ResourceHelp
     assert(!error.hasSparkThrowable)
     assert(error.getMessage == "Regular runtime exception")
   }
+
+  test("throwableToFetchErrorDetailsResponse with null getMessageParameters") {
+    // A SparkThrowable override that returns null from getMessageParameters 
(in violation
+    // of the interface's implicit non-null contract) must not crash the 
FetchErrorDetails
+    // RPC with a NullPointerException.
+    val testError = new Exception("error with null message params") with 
SparkThrowable {
+      override def getCondition: String = "NULL_PARAMS_ERROR"
+      override def getErrorClass: String = "NULL_PARAMS_ERROR"
+      override def getMessageParameters: java.util.Map[String, String] = null
+    }
+
+    val response =
+      ErrorUtils.throwableToFetchErrorDetailsResponse(testError, 
serverStackTraceEnabled = false)
+
+    assert(response.hasRootErrorIdx)
+    val sparkThrowableProto = response.getErrors(0).getSparkThrowable
+    assert(sparkThrowableProto.getErrorClass == "NULL_PARAMS_ERROR")
+    assert(sparkThrowableProto.getMessageParametersMap.isEmpty)
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to