Skip to content

Propagate serialization failures in Resource.serializeObject() instead of swallowing them#5312

Open
gyowoo1113 wants to merge 1 commit into
apache:masterfrom
gyowoo1113:ZEPPELIN-6467-propagate-serialization-failure
Open

Propagate serialization failures in Resource.serializeObject() instead of swallowing them#5312
gyowoo1113 wants to merge 1 commit into
apache:masterfrom
gyowoo1113:ZEPPELIN-6467-propagate-serialization-failure

Conversation

@gyowoo1113

Copy link
Copy Markdown
Contributor

What is this PR for?

Resource.serializeObject(Object) caught serialization exceptions, printed the stack trace, and still returned a ByteBuffer from the partially written output. When serialization failed, callers could receive truncated or empty data instead of the original IOException.

This PR removes the exception-swallowing catch block and uses try-with-resources for ObjectOutputStream. Serialization failures now propagate as the already-declared IOException, allowing the existing caller-side error handling to run and preventing invalid buffers from being returned.

A unit test was added with a Serializable object that throws IOException during serialization. The method signature and behavior for valid serializable objects are unchanged.

What type of PR is it?

Bug Fix

Todos

  • Remove the exception-swallowing catch block
  • Remove printStackTrace()
  • Propagate serialization failures as IOException
  • Use try-with-resources for ObjectOutputStream
  • Add a regression test for serialization failure

What is the Jira issue?

ZEPPELIN-6467

How should this be tested?

./mvnw test -pl zeppelin-interpreter -Dtest=ResourceTest passes successfully.

To verify that the shaded interpreter JAR includes the change:

./mvnw clean package -pl zeppelin-interpreter,zeppelin-interpreter-shaded -DskipTests passes successfully.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant