Skip to content

[ZEPPELIN-6543] Handle invokeMethod serialization failure as InterpreterRPCException - #5349

Open
gyowoo1113 wants to merge 2 commits into
apache:masterfrom
gyowoo1113:ZEPPELIN-6543-handle-invoke-method-serialization-failure
Open

[ZEPPELIN-6543] Handle invokeMethod serialization failure as InterpreterRPCException#5349
gyowoo1113 wants to merge 2 commits into
apache:masterfrom
gyowoo1113:ZEPPELIN-6543-handle-invoke-method-serialization-failure

Conversation

@gyowoo1113

Copy link
Copy Markdown
Contributor

What is this PR for?

This PR follows up on ZEPPELIN-6467 / PR #5312.

Following PR #5312, Resource.serializeObject() propagates serialization failures as IOException. As a result, RemoteInterpreterEventServer.invokeMethod() can receive an exception while re-serializing a remote resource invocation result. The existing handler logged the exception and returned a null ByteBuffer, causing the generated Thrift client to report a missing result instead of preserving the original serialization failure.

The Jira issue identified this behavior through code analysis, but the server-side deserialize-and-re-serialize failure path had not yet been reproduced.

This PR adds a regression test using a serializable object that succeeds during the initial serialization and fails during the server-side second serialization. It then changes invokeMethod() to propagate the failure as InterpreterRPCException, allowing the original error message to reach the caller instead of being converted into an unrelated Thrift missing-result error.

The behavior for successfully serialized results is unchanged.

What type of PR is it?

Bug Fix

Todos

  • Reproduce the server-side deserialize-and-re-serialize failure path with a regression test
  • Propagate serialization failures as InterpreterRPCException
  • Verify that the propagated exception contains the original failure message

What is the Jira issue?

[ZEPPELIN-6543]

How should this be tested?

./mvnw test -pl zeppelin-server -Dtest=RemoteInterpreterEventServerTest 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
  • Code inspection suggests that Resource.serializeObject() may return null when the result is not serializable, but this path is not covered by the regression test in this PR. Should this case also be handled in this PR, or should it be addressed separately?

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