Fix Kotlin Upload Progress Reporting and Error Handling #979
Conversation
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
oguzkocer
force-pushed
the
fix/kotlin-upload-progress-and-error-handling
branch
from
be9e1e2 to
864b5a6
Compare
Add test to verify that upload progress callbacks are invoked correctly and that progress values are monotonically increasing. While this test uses the /media endpoint which only supports single file uploads, it validates the basic progress reporting behavior and documents expected behavior for future multi-file upload scenarios.
Wrap the entire multipart body with ProgressRequestBody instead of wrapping individual files. This ensures progress is cumulative across all files rather than resetting for each file. Before: With multiple files, progress would reset to 0% for each file After: Progress smoothly increases from 0% to 100% across all files Changes: - Remove getRequestBody() helper method - Build complete multipart body first - Wrap entire body with ProgressRequestBody for cumulative tracking
Add try-catch blocks to handle network exceptions (SSL errors, unknown host, no route to host) in the upload() method, matching the exception handling pattern used in execute(). Without this, upload errors would throw unhandled exceptions instead of being properly wrapped in RequestExecutionException.
Add User-Agent header to upload requests to match the behavior of the execute() method. This ensures uploads are properly identified with the client version information.
Ensure HttpsURLConnection is properly closed after certificate inspection to prevent resource leaks. Wrap the certificate extraction in try-finally block to guarantee cleanup, and add exception handling to gracefully handle cases where certificate inspection fails.
Use header() instead of addHeader() when setting User-Agent to ensure it cannot be overridden by headers from the request. The header() method replaces any existing header with the same name, while addHeader() would append and potentially allow unwanted values.
Refactor upload() method to reduce complexity and length by extracting helper methods: - buildMultipartBody(): Builds the multipart form body - wrapWithProgressTracking(): Wraps body with progress tracking - buildUploadRequest(): Constructs the HTTP request - executeUpload(): Executes the upload and returns response Also fix detekt warnings: - Remove unused RequestBody import - Remove extra blank line - Add suppression annotations for SSL error handler with detailed justification comments This reduces the upload() method from 67 lines to 17 lines and improves maintainability.
Consolidate duplicate code between execute() and upload() methods by extracting shared functionality into helper methods: - addRequestHeaders(): Handles adding custom headers and User-Agent for both request types, ensuring consistent header handling - executeRequestSafely(): Centralizes exception handling, call execution, and response building with optional upload listener notification This eliminates code duplication and ensures both methods handle errors, headers, and responses identically. The upload listener is now correctly notified with the actual Call object that gets executed. Changes: - Removed buildUploadRequest() and executeUpload() helper methods - Both execute() and upload() now use common helpers - Reduced overall code size and improved maintainability - Added @Suppress("ThrowsCount") for exception handling (3 specific network exceptions need to be caught and converted)
oguzkocer
force-pushed
the
fix/kotlin-upload-progress-and-error-handling
branch
from
864b5a6 to
24dc0ea
Compare
adalpari
approved these changes
Oct 28, 2025
Contributor
adalpari
left a comment
adalpari
left a comment
There was a problem hiding this comment.
LGTM. I've tested it on the Android app, and both uploading and progress seem to be working as expected.
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.
This PR fixes several issues in the Kotlin
WpRequestExecutorimplementation:RequestExecutionExceptionupload()method to reduce complexity and fix detekt warningsexecute()andupload()methods