Handle errors in chunk uploads (#3899)

This commit is contained in:
Vivek Maskara 2020-08-17 04:43:36 -07:00 committed by GitHub
parent 1856196851
commit 74720aac19
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 99 additions and 14 deletions

View file

@ -3,6 +3,9 @@ package fr.free.nrw.commons;
import androidx.annotation.NonNull;
import java.io.File;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import okhttp3.Cache;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
@ -62,6 +65,8 @@ public final class OkHttpConnectionFactory {
}
public static class UnsuccessfulResponseInterceptor implements Interceptor {
private static final List<String> DO_NOT_INTERCEPT = Collections.singletonList(
"api.php?format=json&formatversion=2&errorformat=plaintext&action=upload&ignorewarnings=1");
private static final String ERRORS_PREFIX = "{\"error";
@ -69,6 +74,11 @@ public final class OkHttpConnectionFactory {
@NonNull
public Response intercept(@NonNull final Chain chain) throws IOException {
final Response rsp = chain.proceed(chain.request());
// Do not intercept certain requests and let the caller handle the errors
if(isExcludedUrl(chain.request())) {
return rsp;
}
if (rsp.isSuccessful()) {
try (final ResponseBody responseBody = rsp.peekBody(ERRORS_PREFIX.length())) {
if (ERRORS_PREFIX.equals(responseBody.string())) {
@ -83,6 +93,16 @@ public final class OkHttpConnectionFactory {
}
throw new HttpStatusException(rsp);
}
private boolean isExcludedUrl(final Request request) {
final String requestUrl = request.url().toString();
for(final String url: DO_NOT_INTERCEPT) {
if(requestUrl.contains(url)) {
return true;
}
}
return false;
}
}
private OkHttpConnectionFactory() {

View file

@ -3,21 +3,23 @@ package fr.free.nrw.commons.contributions
import android.os.Parcel
import android.os.Parcelable
import fr.free.nrw.commons.upload.UploadResult
import kotlinx.android.parcel.Parcelize
data class ChunkInfo(
val uploadResult: UploadResult,
val lastChunkIndex: Int
val lastChunkIndex: Int,
var isLastChunkUploaded: Boolean
) : Parcelable {
constructor(parcel: Parcel) : this(
parcel.readParcelable(UploadResult::class.java.classLoader),
parcel.readInt()
parcel.readInt(),
parcel.readByte() != 0.toByte()
) {
}
override fun writeToParcel(parcel: Parcel, flags: Int) {
parcel.writeParcelable(uploadResult, flags)
parcel.writeInt(lastChunkIndex)
parcel.writeByte(if (isLastChunkUploaded) 1 else 0)
}
override fun describeContents(): Int {

View file

@ -5,13 +5,13 @@ import static fr.free.nrw.commons.di.NetworkingModule.NAMED_COMMONS_CSRF;
import android.content.Context;
import android.net.Uri;
import androidx.annotation.Nullable;
import com.google.gson.Gson;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.contributions.ChunkInfo;
import fr.free.nrw.commons.contributions.Contribution;
import fr.free.nrw.commons.upload.UploadService.NotificationUpdateProgressListener;
import io.reactivex.Observable;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.functions.Consumer;
import java.io.File;
import java.io.IOException;
import java.util.Date;
@ -24,6 +24,7 @@ import okhttp3.MediaType;
import okhttp3.MultipartBody;
import okhttp3.RequestBody;
import org.wikipedia.csrf.CsrfTokenClient;
import org.wikipedia.dataclient.mwapi.MwException;
import timber.log.Timber;
@Singleton
@ -39,6 +40,7 @@ public class UploadClient {
private final CsrfTokenClient csrfTokenClient;
private final PageContentsCreator pageContentsCreator;
private final FileUtilsWrapper fileUtilsWrapper;
private final Gson gson;
private boolean pauseUploads = false;
private final CompositeDisposable compositeDisposable = new CompositeDisposable();
@ -47,11 +49,12 @@ public class UploadClient {
public UploadClient(final UploadInterface uploadInterface,
@Named(NAMED_COMMONS_CSRF) final CsrfTokenClient csrfTokenClient,
final PageContentsCreator pageContentsCreator,
final FileUtilsWrapper fileUtilsWrapper) {
final FileUtilsWrapper fileUtilsWrapper, final Gson gson) {
this.uploadInterface = uploadInterface;
this.csrfTokenClient = csrfTokenClient;
this.pageContentsCreator = pageContentsCreator;
this.fileUtilsWrapper = fileUtilsWrapper;
this.gson = gson;
}
/**
@ -61,6 +64,10 @@ public class UploadClient {
Observable<StashUploadResult> uploadFileToStash(
final Context context, final String filename, final Contribution contribution,
final NotificationUpdateProgressListener notificationUpdater) throws IOException {
if (contribution.getChunkInfo() != null && contribution.getChunkInfo().isLastChunkUploaded()) {
return Observable.just(new StashUploadResult(StashUploadState.SUCCESS,
contribution.getChunkInfo().getUploadResult().getFilekey()));
}
pauseUploads = false;
File file = new File(contribution.getLocalUri().getPath());
final Observable<File> fileChunks = fileUtilsWrapper.getFileChunks(context, file, CHUNK_SIZE);
@ -69,6 +76,7 @@ public class UploadClient {
final AtomicInteger index = new AtomicInteger();
final AtomicReference<ChunkInfo> chunkInfo = new AtomicReference<>();
Timber.d("Chunk info");
if (contribution.getChunkInfo() != null && isStashValid(contribution)) {
chunkInfo.set(contribution.getChunkInfo());
}
@ -96,12 +104,15 @@ public class UploadClient {
offset,
filekey,
countingRequestBody).subscribe(uploadResult -> {
chunkInfo.set(new ChunkInfo(uploadResult, index.incrementAndGet()));
chunkInfo.set(new ChunkInfo(uploadResult, index.incrementAndGet(), false));
notificationUpdater.onChunkUploaded(contribution, chunkInfo.get());
}, throwable -> {
Timber.e(throwable, "Error occurred in uploading chunk");
}));
}));
chunkInfo.get().setLastChunkUploaded(true);
notificationUpdater.onChunkUploaded(contribution, chunkInfo.get());
if (pauseUploads) {
return Observable.just(new StashUploadResult(StashUploadState.PAUSED, null));
} else if (chunkInfo.get() != null) {
@ -183,9 +194,16 @@ public class UploadClient {
pageContentsCreator.createFrom(contribution),
CommonsApplication.DEFAULT_EDIT_SUMMARY,
uniqueFileName,
fileKey).map(UploadResponse::getUpload);
fileKey).map(uploadResponse -> {
UploadResponse uploadResult = gson.fromJson(uploadResponse, UploadResponse.class);
if (uploadResult.getUpload() == null) {
final MwException exception = gson.fromJson(uploadResponse, MwException.class);
throw new RuntimeException(exception.getErrorCode());
}
return uploadResult.getUpload();
});
} catch (final Throwable throwable) {
throwable.printStackTrace();
Timber.e(throwable, "Exception occurred in uploading file from stash");
return Observable.error(throwable);
}
}

View file

@ -2,6 +2,7 @@ package fr.free.nrw.commons.upload;
import androidx.annotation.NonNull;
import com.google.gson.JsonObject;
import io.reactivex.Observable;
import okhttp3.MultipartBody;
import okhttp3.RequestBody;
@ -29,7 +30,7 @@ public interface UploadInterface {
@POST(MW_API_PREFIX + "action=upload&ignorewarnings=1")
@FormUrlEncoded
@NonNull
Observable<UploadResponse> uploadFileFromStash(@NonNull @Field("token") String token,
Observable<JsonObject> uploadFileFromStash(@NonNull @Field("token") String token,
@NonNull @Field("text") String text,
@NonNull @Field("comment") String comment,
@NonNull @Field("filename") String filename,

View file

@ -26,11 +26,14 @@ import fr.free.nrw.commons.wikidata.WikidataEditService;
import io.reactivex.Observable;
import io.reactivex.Scheduler;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.functions.Consumer;
import io.reactivex.processors.PublishProcessor;
import io.reactivex.schedulers.Schedulers;
import java.io.IOException;
import java.util.Arrays;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@ -42,6 +45,9 @@ public class UploadService extends CommonsDaggerService {
private static final String EXTRA_PREFIX = "fr.free.nrw.commons.upload";
private static final List<String> STASH_ERROR_CODES = Arrays
.asList("uploadstash-file-not-found", "stashfailed", "verification-error", "chunk-too-small");
public static final String ACTION_START_SERVICE = EXTRA_PREFIX + ".upload";
public static final String EXTRA_FILES = EXTRA_PREFIX + ".files";
@Inject
@ -290,7 +296,15 @@ public class UploadService extends CommonsDaggerService {
getApplicationContext(),
contribution,
uniqueFilename,
uploadStash.getFileKey());
uploadStash.getFileKey()).doOnError(new Consumer<Throwable>() {
@Override
public void accept(Throwable throwable) throws Exception {
Timber.e(throwable, "Error occurred in uploading file from stash");
if (STASH_ERROR_CODES.contains(throwable.getMessage())) {
clearChunks(contribution);
}
}
});
} else if (uploadStash.getState() == StashUploadState.PAUSED) {
Timber.d("Contribution upload paused");
showPausedNotification(contribution);
@ -310,6 +324,13 @@ public class UploadService extends CommonsDaggerService {
});
}
private void clearChunks(Contribution contribution) {
contribution.setChunkInfo(null);
compositeDisposable.add(contributionDao.update(contribution)
.subscribeOn(ioThreadScheduler)
.subscribe());
}
private void onUpload(Contribution contribution, String notificationTag,
UploadResult uploadResult) {
Timber.d("Stash upload response 2 is %s", uploadResult.toString());

View file

@ -2,15 +2,32 @@ package org.wikipedia.dataclient.mwapi;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import java.util.List;
public class MwException extends RuntimeException {
@SuppressWarnings("unused") @NonNull private final MwServiceError error;
@SuppressWarnings("unused") @Nullable private final MwServiceError error;
public MwException(@NonNull MwServiceError error) {
@SuppressWarnings("unused") @Nullable private final List<MwServiceError> errors;
public MwException(@Nullable MwServiceError error,
@Nullable final List<MwServiceError> errors) {
this.error = error;
this.errors = errors;
}
@NonNull public MwServiceError getError() {
@NonNull
public List<MwServiceError> getErrors() {
return errors;
}
public String getErrorCode() {
if(error!=null) {
return error.getCode();
}
return errors != null ? errors.get(0).getCode() : null;
}
@Nullable public MwServiceError getError() {
return error;
}

View file

@ -17,7 +17,7 @@ public abstract class MwResponse extends BaseModel implements PostProcessingType
@Override
public void postProcess() {
if (errors != null && !errors.isEmpty()) {
throw new MwException(errors.get(0));
throw new MwException(errors.get(0), errors);
}
}
}

View file

@ -13,6 +13,7 @@ import java.util.List;
* Gson POJO for a MediaWiki API error.
*/
public class MwServiceError extends BaseModel implements ServiceError {
@SuppressWarnings("unused") @Nullable private String code;
@SuppressWarnings("unused") @Nullable private String text;
@SuppressWarnings("unused") @Nullable private Data data;
@ -55,6 +56,11 @@ public class MwServiceError extends BaseModel implements ServiceError {
return null;
}
@Nullable
public String getCode() {
return code;
}
private static final class Data {
@SuppressWarnings("unused") @Nullable private List<Message> messages;