Fix: Indicate correct error message in case of loss of internet conne… by 20020316 · Pull Request #6736 · commons-app/apps-android-commons

20020316 · 2026-03-14T12:42:53Z

…ctivity on Android 14(#6311).

Description (required)

What changes did you make and why?
Implemented appropriate error messages with the onErrorResumeNext method from the JavaRx library. Previously, all errors were caught in a single try/catch block, regardless of type, with no distinction between network, token, or other unexpected errors. The onErrorResumeNext operator allows each error type to be handled separately with a specific message, sending them back to Observable.error(e) . This, in turn, also forces some other changes, namely setting explicitly nullable variables, e.g UploadResult -> UploadResult? rather than leaving them to be non-nullable.

Tests performed (required)

Tested {build variant, e.g. ProdDebug} on {name of device or emulator} with API level {API level}.

Tested on motorola edge 60, Android 16, API 36 and xiaomi redmi note 12 pro Android 14 API 34.

Screenshots (for UI changes only)

Need help? See https://support.google.com/android/answer/9075928

Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

…ctivity on Android 14(commons-app#6311). Implemented appropriate error messages with the JavaRx library for network errors that occur during uploads, as previously the error messages were wrong. Try/catch did not work except for missing token exceptions.

gradle/wrapper/gradle-wrapper.properties

…ctivity on Android 14(commons-app#6311). Removed gradle wrapper properties change and indentation changes from UploadClient.kt.

Kota-Jagadeesh

@20020316 Could you fill in the remaining information in the PR description?

20020316 · 2026-03-14T21:09:40Z

@Kota-Jagadeesh @RitikaPahwa4444 updated the PR. Could the pull request be approved now?

RitikaPahwa4444 · 2026-03-14T22:40:06Z

I'm not sure if it's just me but GitHub isn't showing any further commits 😕

20020316 · 2026-03-14T22:55:17Z

@RitikaPahwa4444 Now it should be visible.

gradle/wrapper/gradle-wrapper.properties

Date fixed.

RitikaPahwa4444 · 2026-03-15T08:03:34Z

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

                        uploadResult.upload
+                    }.onErrorResumeNext { e: Throwable ->
+                        Timber.e(e, when (e) {
+                            is InvalidLoginTokenException -> "The server could not retrieve the session token."


How about writing a helper function for this common code?

Yes, agreed.

RitikaPahwa4444 · 2026-03-15T08:05:03Z

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

                else -> {
                    Timber.d("Upload stash failed %s", contribution.pageId)
-                    Observable.just(StashUploadResult(StashUploadState.FAILED, null, null))
+                    Observable.just(StashUploadResult(StashUploadState.FAILED, null, "Network error"))


Are we sure it fails here due to a network error only?

No, I do not know why it remained a network error. I had reversed it. I will commit again with null instead.

RitikaPahwa4444 · 2026-03-15T08:07:15Z

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

                    countingRequestBody,
                ).subscribe(
-                    { uploadResult: UploadResult ->
+                    { uploadResult: UploadResult? ->


uploadChunkToStash returns Observable<UploadResult>. It's not a nullable, is there anything that I'm missing out?

So the uploadChunkToStash returned Observable<uploadResult> previously because the try catch block handled all exceptions as one, rather than dealing with them separately, so the compiler did not complain. The current code explicitly handles errors; it's better to handle the null exception separately from network errors. And to add, uploadChunksToStash maps its result to a variable of type UploadResult?, which is declared as such in UploadResponse.

I noticed this PR adds onErrorResumeNext to the code, which makes it a nullable. Could you please elaborate in the PR description why this is necessary to fix the issue?

I added the modified PR description per your request.

RitikaPahwa4444 · 2026-03-15T08:07:56Z

gradle/wrapper/gradle-wrapper.properties

 distributionUrl=https\://services.gradle.org/distributions/gradle-8.13-bin.zip
 zipStoreBase=GRADLE_USER_HOME
-zipStorePath=wrapper/dists
+zipStorePath=wrapper/dists


Newline got removed/added, is it? GitHub is showing a diff here, so just confirming.

I am unsure why the diff shows here, but I think it might be because I removed the initial line with the 2026 date, then added a new one at the top afterwards when reverting back to the 2023 date.

Copilot

Pull request overview

Improves upload failure reporting by distinguishing network/token/timeout errors during chunked stash uploads and final “upload from stash”, addressing misleading error messages when connectivity is lost (Issue #6311).

Changes:

Refactors upload error handling to classify common network/token exceptions and surface more appropriate messages.
Updates upload APIs/nullability around UploadResult and adds onErrorResumeNext logging/propagation.
Minor formatting normalization in Gradle wrapper properties.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`gradle/wrapper/gradle-wrapper.properties`	Minor properties formatting/line normalization.
`app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt`	Adds per-exception messaging and refactors Rx error handling for stash chunk upload + upload-from-stash.

Comments suppressed due to low confidence (1)

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt:286

uploadFileFromStash now returns Observable<UploadResult> but still force-unwraps uniqueFileName!!/fileKey!! and calls csrfTokenClient.getTokenBlocking() eagerly. These can throw before an Observable is created, bypassing Rx error handling. Also, existing call sites handle errors by returning null from onErrorReturn, which will crash at runtime with RxJava (null emissions are not allowed) if this method’s contract is now non-null. Either (a) keep the return type nullable and handle null consistently, or (b) make inputs non-null + wrap creation in Observable.defer { ... } and update call sites to handle errors via onErrorReturnItem/onErrorResumeNext without emitting null.

        fun uploadFileFromStash(
            contribution: Contribution?,
            uniqueFileName: String?,
            fileKey: String?,
        ): Observable<UploadResult> =
                uploadInterface
                    .uploadFileFromStash(
                        csrfTokenClient.getTokenBlocking(),
                        pageContentsCreator.createFrom(contribution),
                        CommonsApplication.DEFAULT_EDIT_SUMMARY,
                        uniqueFileName!!,
                        fileKey!!,
                    ).map { uploadResponse: JsonObject? ->

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

+                .map(UploadResponse::upload).onErrorResumeNext { e: Throwable ->
+                    Timber.e(e, when (e) {
+                        is InvalidLoginTokenException -> "The server could not retrieve the session token."
+                        is UnknownHostException  -> "Could not contact the remote host."
+                        is SocketTimeoutException -> "The socket operation timed out while uploading."
+                        is ConnectException  -> "The connection was interrupted while uploading a chunk."
+                        else -> "Unexpected error during chunk upload."
+                    })
+                    Observable.error(e)
+                }


app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

20020316 · 2026-03-15T10:54:00Z

@RitikaPahwa4444 Thank you for the GitHub Copilot review. I saw my mistakes and am working towards addressing them.

…ctivity on Android 14(commons-app#6311). Added requested changes following review and testing(helper function, copilot review)

20020316 · 2026-03-15T13:53:36Z

@RitikaPahwa4444 Pushed the changes requested right now, so both the changes indicated by the copilot review along with the helper function.

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

Indentation changes reverted.

…ctivity on Android 14(commons-app#6311). Added requested changes following review and testing(helper function, copilot review)

# Conflicts: # app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

Newline fixed.

20020316 · 2026-03-15T15:04:09Z

@RitikaPahwa4444 It should be fine now, added strings to the xml file for localization(did not remove function since the class where it is used does not extend Context nor Activity). Indentation also fixed.

RitikaPahwa4444 · 2026-03-15T15:16:32Z

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

-                        URLEncoder.encode(filename, "utf-8"),
-                        countingRequestBody,
-                    )
+    fun uploadChunkToStash(


All the other functions seem to have additional space on the left, would you mind checking the indentation once?

Fixed, uniform indentation in the file now.

@20020316, instead of changing the indentation of the entire file, would it be possible to fix it just for the function you wished to change? The diff is really hard to go through.

@RitikaPahwa4444 I reverted the indentation to the original version, that is, the unmodified version of the UploadClient.kt file, before the first commit. I hope it's fine now, as I assume you want to keep the indentation of the original, not my own, along with the added functionality for handling errors.

I think it should be far easier to analyse the difference between the original and the new file now.

@RitikaPahwa4444 The commit diff should now be clear. I fixed all indentation diff. Please check. If all is good, can the commits be approved for merge with the main branch and the issue closed?

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

…urce access for strings.(commons-app#6311)

Fixed an inadvertently changed string.

20020316 · 2026-03-16T18:28:00Z

@RitikaPahwa4444 @Kota-Jagadeesh Could you please review the latest changes and, if all is ok, approve the merge and close the issue?

Fixed newline in gradle.properties once more.

…vate values.

Final indentation fix for UploadClient.kt

Final indentation fix.

RitikaPahwa4444 reviewed Mar 14, 2026

View reviewed changes

gradle/wrapper/gradle-wrapper.properties Outdated Show resolved Hide resolved

RaresNicoMoldo and others added 4 commits March 14, 2026 17:02

Fix: Indicate correct error message in case of loss of internet conne…

13d32ec

…ctivity on Android 14(commons-app#6311). Removed gradle wrapper properties change and indentation changes from UploadClient.kt.

Fix: Indicate correct error message in case of loss of internet conne…

5489899

…ctivity on Android 14(commons-app#6311). Removed gradle wrapper properties change and indentation changes from UploadClient.kt.

Merge branch 'commons-app:main' into main

794eb37

Merge branch 'main' of https://github.com/20020316/apps-android-commons

0d56317

Kota-Jagadeesh reviewed Mar 14, 2026

View reviewed changes

RitikaPahwa4444 reviewed Mar 14, 2026

View reviewed changes

gradle/wrapper/gradle-wrapper.properties Outdated Show resolved Hide resolved

Update gradle-wrapper.properties

455c1ba

Date fixed.

RitikaPahwa4444 requested changes Mar 15, 2026

View reviewed changes

RitikaPahwa4444 requested a review from Copilot March 15, 2026 10:32

Copilot started reviewing on behalf of RitikaPahwa4444 March 15, 2026 10:32 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

RaresNicoMoldo added 2 commits March 15, 2026 14:50

Fix: Indicate correct error message in case of loss of internet conne…

55d76f8

…ctivity on Android 14(commons-app#6311). Added requested changes following review and testing(helper function, copilot review)

Merge branch 'main' of https://github.com/20020316/apps-android-commons

80bbc1b

RitikaPahwa4444 requested changes Mar 15, 2026

View reviewed changes

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt Outdated Show resolved Hide resolved

app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt Outdated Show resolved Hide resolved

20020316 and others added 4 commits March 15, 2026 15:57

Update UploadClient.kt

6c1c992

Indentation changes reverted.

Fix: Indicate correct error message in case of loss of internet conne…

84c0804

…ctivity on Android 14(commons-app#6311). Added requested changes following review and testing(helper function, copilot review)

Merge branch 'main' of https://github.com/20020316/apps-android-commons

0fd4a9e

# Conflicts: # app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt

Update gradle-wrapper.properties

1fc9ab1

Newline fixed.

RitikaPahwa4444 requested changes Mar 15, 2026

View reviewed changes

RaresNicoMoldo and others added 2 commits March 15, 2026 19:08

Added strings to xml, uniform indentation, and context added for reso…

a7fce6d

…urce access for strings.(commons-app#6311)

Update strings.xml

182a6fb

Fixed an inadvertently changed string.

20020316 and others added 6 commits March 17, 2026 13:49

Update gradle-wrapper.properties

459bcea

Fixed newline in gradle.properties once more.

Reverted indentation to original(unmodified) file indentation.

a860667

Merge branch 'main' of https://github.com/20020316/apps-android-commons

aeffd86

Indentation fix for uploadFileToStash, class parameters and class pri…

65a5569

…vate values.

Update UploadClient.kt

1f730b7

Final indentation fix for UploadClient.kt

Update UploadClient.kt

6dcc6ec

Final indentation fix.

20020316 requested a review from RitikaPahwa4444 March 20, 2026 14:59

Conversation

20020316 commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Kota-Jagadeesh left a comment

Choose a reason for hiding this comment

Uh oh!

20020316 commented Mar 14, 2026

Uh oh!

RitikaPahwa4444 commented Mar 14, 2026

Uh oh!

20020316 commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RitikaPahwa4444 Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

20020316 commented Mar 15, 2026

Uh oh!

20020316 commented Mar 15, 2026

Uh oh!

Uh oh!

Uh oh!

20020316 commented Mar 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

20020316 Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

20020316 commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

20020316 commented Mar 14, 2026 •

edited

Loading

20020316 commented Mar 14, 2026 •

edited

Loading

RitikaPahwa4444 Mar 15, 2026 •

edited

Loading

20020316 Mar 17, 2026 •

edited

Loading

20020316 commented Mar 16, 2026 •

edited

Loading