-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix: Indicate correct error message in case of loss of internet conne… #6736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
e8e2c2a
13d32ec
5489899
794eb37
0d56317
455c1ba
55d76f8
80bbc1b
6c1c992
84c0804
0fd4a9e
1fc9ab1
a7fce6d
182a6fb
459bcea
a860667
aeffd86
65a5569
1f730b7
6dcc6ec
31fb5be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| package fr.free.nrw.commons.upload | ||
|
|
||
|
|
||
| import com.google.gson.Gson | ||
| import com.google.gson.JsonObject | ||
| import fr.free.nrw.commons.CommonsApplication | ||
| import fr.free.nrw.commons.auth.csrf.CsrfTokenClient | ||
| import fr.free.nrw.commons.auth.csrf.InvalidLoginTokenException | ||
| import fr.free.nrw.commons.contributions.ChunkInfo | ||
| import fr.free.nrw.commons.contributions.Contribution | ||
| import fr.free.nrw.commons.contributions.ContributionDao | ||
|
|
@@ -22,15 +24,17 @@ import okhttp3.RequestBody.Companion.toRequestBody | |
| import timber.log.Timber | ||
| import java.io.File | ||
| import java.io.IOException | ||
| import java.net.ConnectException | ||
| import java.net.SocketTimeoutException | ||
| import java.net.URLEncoder | ||
| import java.net.UnknownHostException | ||
| import java.util.Date | ||
| import java.util.concurrent.atomic.AtomicBoolean | ||
| import java.util.concurrent.atomic.AtomicInteger | ||
| import java.util.concurrent.atomic.AtomicReference | ||
| import javax.inject.Inject | ||
| import javax.inject.Named | ||
| import javax.inject.Singleton | ||
|
|
||
| @Singleton | ||
| class UploadClient | ||
| @Inject | ||
|
|
@@ -134,7 +138,7 @@ class UploadClient | |
| } | ||
| else -> { | ||
| Timber.d("Upload stash failed %s", contribution.pageId) | ||
| Observable.just(StashUploadResult(StashUploadState.FAILED, null, null)) | ||
| Observable.just(StashUploadResult(StashUploadState.FAILED, null, "Network error")) | ||
20020316 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
|
|
@@ -178,18 +182,28 @@ class UploadClient | |
| filekey, | ||
| countingRequestBody, | ||
| ).subscribe( | ||
| { uploadResult: UploadResult -> | ||
| { uploadResult: UploadResult? -> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed this PR adds
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the modified PR description per your request. |
||
| Timber.d( | ||
| "Chunk: Received Chunk number: %s, offset: %s", | ||
| index.get(), | ||
| uploadResult.offset, | ||
| uploadResult?.offset, | ||
| ) | ||
| chunkInfo.set(ChunkInfo(uploadResult, index.get(), totalChunks)) | ||
| notificationUpdater.onChunkUploaded(contribution, chunkInfo.get()) | ||
| }, | ||
| { throwable: Throwable? -> | ||
| Timber.e(throwable, "Received error in chunk upload") | ||
| errorMessage.set(throwable?.message) | ||
|
|
||
| val message = when (throwable) { | ||
| is InvalidLoginTokenException ->"The session token was not found by the"+ | ||
| " server." | ||
| is UnknownHostException -> "Could not contact the remote host." | ||
| is ConnectException -> "The connection was interrupted while uploading." | ||
| is SocketTimeoutException -> "The socket operation timed out." | ||
| else -> throwable?.message ?: "An unexpected error occurred." | ||
| } | ||
|
|
||
| errorMessage.set(message) | ||
| failures.set(true) | ||
| }, | ||
| ), | ||
|
|
@@ -226,28 +240,33 @@ class UploadClient | |
| offset: Long, | ||
| fileKey: String?, | ||
| countingRequestBody: CountingRequestBody, | ||
| ): Observable<UploadResult> { | ||
| val filePart: MultipartBody.Part | ||
| return try { | ||
| filePart = | ||
| MultipartBody.Part.createFormData( | ||
| "chunk", | ||
| URLEncoder.encode(filename, "utf-8"), | ||
| countingRequestBody, | ||
| ) | ||
| uploadInterface | ||
| .uploadFileToStash( | ||
| toRequestBody(filename), | ||
| toRequestBody(fileSize.toString()), | ||
| toRequestBody(offset.toString()), | ||
| toRequestBody(fileKey), | ||
| toRequestBody(csrfTokenClient.getTokenBlocking()), | ||
| filePart, | ||
| ).map(UploadResponse::upload) | ||
| } catch (throwable: Throwable) { | ||
| Timber.e(throwable, "Failed to upload chunk to stash") | ||
| Observable.error(throwable) | ||
| } | ||
| ): Observable<UploadResult?> { | ||
| val filePart = MultipartBody.Part.createFormData( | ||
| "chunk", | ||
| URLEncoder.encode(filename, "utf-8"), | ||
| countingRequestBody | ||
| ) | ||
20020316 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return uploadInterface | ||
| .uploadFileToStash( | ||
| toRequestBody(filename), | ||
| toRequestBody(fileSize.toString()), | ||
| toRequestBody(offset.toString()), | ||
| toRequestBody(fileKey), | ||
| toRequestBody(csrfTokenClient.getTokenBlocking()), | ||
| filePart | ||
| ) | ||
| .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) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -260,7 +279,6 @@ class UploadClient | |
| uniqueFileName: String?, | ||
| fileKey: String?, | ||
| ): Observable<UploadResult?> = | ||
| try { | ||
| uploadInterface | ||
| .uploadFileFromStash( | ||
| csrfTokenClient.getTokenBlocking(), | ||
|
|
@@ -276,11 +294,17 @@ class UploadClient | |
| throw Exception(exception.errorCode) | ||
| } | ||
| uploadResult.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 connection timed out." | ||
| is ConnectException -> "The connection was interrupted while uploading." | ||
| else -> "An unexpected error occurred." | ||
| }) | ||
| Observable.error(e) | ||
| } | ||
| } catch (throwable: Throwable) { | ||
| Timber.e(throwable, "Exception occurred in uploading file from stash") | ||
| Observable.error(throwable) | ||
| } | ||
| } | ||
|
|
||
| private fun canProcess( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure it fails here due to a network error only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I do not know why it remained a network error. I had reversed it. I will commit again with null instead.