Skip to content

Conversation

@jshmchenxi
Copy link
Contributor

What changes were proposed in this pull request?

Handle null input for SparkThrowableHelper.isInternalError method.

Why are the changes needed?

The SparkThrowableHelper.isInternalError method doesn't handle null input, and it could lead to NullPointerException. It happens when a SparkException without errorClass is invoked isInternalError.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add 2 assertions to current test cases to cover this issue.

Was this patch authored or co-authored using generative AI tooling?

No

…er.isInternalError`

The SparkThrowableHelper.isInternalError method doesn't handle null input, and it could lead to NullPointerException.

Example stacktrace from our environment with Spark 3.5.1:

```
 Caused by: java.lang.NullPointerException: Cannot invoke "String.startsWith(String)" because "errorClass" is null
    at org.apache.spark.SparkThrowableHelper$.isInternalError(SparkThrowableHelper.scala:64)
    at org.apache.spark.SparkThrowableHelper.isInternalError(SparkThrowableHelper.scala)
    at org.apache.spark.SparkThrowable.isInternalError(SparkThrowable.java:50)
    at org.apache.spark.SparkException.isInternalError(SparkException.scala:27)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:688)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:772)
    ... 30 more
```
@github-actions github-actions bot added the CORE label Aug 31, 2024
@jshmchenxi
Copy link
Contributor Author

Kindly ping @bozhang2820 @cloud-fan. Please take a look, thanks!

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

After your changes, this check st.getErrorClass != null is redundant.

case st: Exception with SparkThrowable if st.getErrorClass != null &&

@jshmchenxi Could your remove it, please.

@jshmchenxi
Copy link
Contributor Author

jshmchenxi commented Sep 2, 2024

After your changes, this check st.getErrorClass != null is redundant.

case st: Exception with SparkThrowable if st.getErrorClass != null &&

@jshmchenxi Could your remove it, please.

@MaxGekk Thanks for review! Removing it will change the condition from false to true when st.getErrorClass is null, which doesn't seem to be right.

Before: st.getErrorClass != null && !SparkThrowableHelper.isInternalError(st.getErrorClass) => false
After: !SparkThrowableHelper.isInternalError(st.getErrorClass) => true

@MaxGekk
Copy link
Member

MaxGekk commented Sep 2, 2024

Thanks for review! Removing it will change the condition from false to true when st.getErrorClass is null, which doesn't seem to be right.

I see, it checks not only internal but not internal AND defined error class. Thank you for checking.

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.5!

@cloud-fan cloud-fan closed this in cef3c86 Sep 2, 2024
cloud-fan pushed a commit that referenced this pull request Sep 2, 2024
…er.isInternalError`

### What changes were proposed in this pull request?

Handle null input for `SparkThrowableHelper.isInternalError` method.

### Why are the changes needed?

The `SparkThrowableHelper.isInternalError` method doesn't handle null input, and it could lead to NullPointerException. It happens when a `SparkException` without `errorClass` is invoked `isInternalError`.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Add 2 assertions to current test cases to cover this issue.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes #47946 from jshmchenxi/SPARK-49480/null-pointer-is-internal-error.

Authored-by: Xi Chen <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit cef3c86)
Signed-off-by: Wenchen Fan <[email protected]>
@jshmchenxi
Copy link
Contributor Author

@cloud-fan Thank you!

turboFei pushed a commit to turboFei/spark that referenced this pull request Nov 6, 2025
…er.isInternalError` (apache#554)

### What changes were proposed in this pull request?

Handle null input for `SparkThrowableHelper.isInternalError` method.

### Why are the changes needed?

The `SparkThrowableHelper.isInternalError` method doesn't handle null input, and it could lead to NullPointerException. It happens when a `SparkException` without `errorClass` is invoked `isInternalError`.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Add 2 assertions to current test cases to cover this issue.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#47946 from jshmchenxi/SPARK-49480/null-pointer-is-internal-error.

Authored-by: Xi Chen <[email protected]>

(cherry picked from commit cef3c86)

Signed-off-by: Wenchen Fan <[email protected]>
Co-authored-by: Xi Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants