fix: surface readable error when server app exception cannot be deserialized#1029
Draft
David-Development wants to merge 1 commit into
Draft
fix: surface readable error when server app exception cannot be deserialized#1029David-Development wants to merge 1 commit into
David-Development wants to merge 1 commit into
Conversation
…ialized The Files app serializes the original exception object over the AIDL pipe. If its class - or any class in its cause chain - only exists in the Files app (e.g. CertificateCombinedException from the owncloud library on SSL errors), ObjectInputStream#readObject throws a ClassNotFoundException in the client app, hiding the actual error behind a message like: java.lang.ClassNotFoundException: com.owncloud.android.lib.common.net... Catch the ClassNotFoundException during deserialization and replace it with a plain exception carrying the original type name and a hint to check the server connection. It flows through the existing parseNextcloudCustomException translation, so client apps get a proper SSOException (UnknownErrorException) instead of a raw ClassNotFoundException. Ref nextcloud/news-android#1645 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 3 minor |
🟢 Metrics 10 complexity
Metric Results Complexity 10
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a request fails inside the Files app,
InputStreamBinderserializes the original exception object over the AIDL pipe. If the exception's class — or any class in its cause chain — only exists in the Files app (e.g.com.owncloud.android.lib.common.network.CertificateCombinedExceptionon SSL/certificate errors),ObjectInputStream#readObjectinAidlNetworkRequest#deserializeObjectV2throws aClassNotFoundExceptionin the client app.The real error is completely lost, and client apps end up showing something like:
See nextcloud/news-android#1645 for a report of exactly this (expired SSL certificate → cryptic
ClassNotFoundExceptionin the News app).Fix
Catch the
ClassNotFoundExceptionduring exception deserialization and replace it with a plain exception that carries the original type name plus a hint to check the server connection. It then flows through the existingSSOException#parseNextcloudCustomExceptiontranslation, so client apps receive a properSSOException(UnknownErrorException) and show their regular error dialog instead of a rawClassNotFoundException.This is the receiver-side safety net and works against all already-shipped Files app versions. Follow-up (separate PR in nextcloud/android): flatten exceptions to plain JRE types in
InputStreamBinder#serializeObjectToInputStreamV2before serializing, so the original error message survives the IPC as well, not just the type name.Tests
Added
AidlNetworkRequestTest:ClassNotFoundException) containing the original type name is returnednullexception + headers are still parsed correctly🤖 Generated with Claude Code