Skip to content

Commit 524b33f

Browse files
nitzanjnitzanj
authored
Refactor max error count handling for upload requests.
1 parent 52f6120 commit 524b33f

File tree

4 files changed

+113
-56
lines changed

4 files changed

+113
-56
lines changed

‎lib/src/androidTest/java/com/cloudinary/android/RequestProcessorTest.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,37 @@ public Boolean call() throws Exception {
181181
});
182182
}
183183

184+
@Test
185+
public void testMaxRetries() throws IOException {
186+
RequestParams params = provideRequestParams();
187+
params.putString("uri", new FilePayload(assetToFile(TEST_IMAGE).getAbsolutePath()).toUri());
188+
HashMap<String, Object> options = new HashMap<>();
189+
// verify that the parameter reaches all the way to the uploader inside:
190+
final String id = UUID.randomUUID().toString();
191+
options.put("public_id", id);
192+
options.put("unsigned", true);
193+
options.put("upload_preset", TEST_PRESET);
194+
params.putString("options", UploadRequest.encodeOptions(options));
195+
params.putInt("errorCount", 15);
196+
197+
DefaultCallbackDispatcher callbackDispatcher = provideCallbackDispatcher();
198+
RequestProcessor processor = provideRequestProcessor(callbackDispatcher);
199+
final StatefulCallback statefulCallback = new StatefulCallback();
200+
callbackDispatcher.registerCallback(statefulCallback);
201+
processor.processRequest(InstrumentationRegistry.getTargetContext(), params);
202+
203+
// wait for result
204+
Awaitility.await().atMost(Duration.TEN_SECONDS).until(new Callable<Boolean>() {
205+
@Override
206+
public Boolean call() {
207+
return statefulCallback.hasResponse();
208+
}
209+
});
210+
211+
assertNotNull(statefulCallback.lastErrorObject);
212+
assertEquals(ErrorInfo.TOO_MANY_ERRORS, statefulCallback.lastErrorObject.getCode());
213+
}
214+
184215
/**
185216
* Bundle based implementation for RequestParams, for testing purposes.
186217
*/

‎lib/src/main/java/com/cloudinary/android/DefaultRequestProcessor.java‎

Lines changed: 80 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.cloudinary.android;
22

33
import android.content.Context;
4+
import android.support.annotation.NonNull;
45

56
import com.cloudinary.Cloudinary;
67
import com.cloudinary.ProgressCallback;
@@ -20,6 +21,7 @@
2021

2122
import java.io.IOException;
2223
import java.io.InterruptedIOException;
24+
import java.util.Locale;
2325
import java.util.Map;
2426
import java.util.concurrent.atomic.AtomicInteger;
2527

@@ -34,6 +36,7 @@
3436
*/
3537
class DefaultRequestProcessor implements RequestProcessor {
3638
private static final String TAG = "DefaultRequestProcessor";
39+
public static final String ERROR_COUNT_PARAM = "errorCount";
3740
private final CallbackDispatcher callbackDispatcher;
3841
private AtomicInteger runningJobs = new AtomicInteger(0);
3942

@@ -51,15 +54,10 @@ public UploadStatus processRequest(Context context, RequestParams params) {
5154
final String uri = params.getString("uri", null);
5255
final String optionsAsString = params.getString("options", null);
5356
final int maxErrorRetries = params.getInt("maxErrorRetries", MediaManager.get().getGlobalUploadPolicy().getMaxErrorRetries());
54-
final int errorCount = params.getInt("errorCount", 0);
57+
final int errorCount = params.getInt(ERROR_COUNT_PARAM, 0);
5558

5659
Logger.i(TAG, String.format("Processing Request %s.", requestId));
5760

58-
if (errorCount > maxErrorRetries) {
59-
Logger.d(TAG, String.format("Failing request %s, too many errors: %d, max: %d", requestId, errorCount, maxErrorRetries));
60-
return FAILURE;
61-
}
62-
6361
callbackDispatcher.dispatchStart(requestId);
6462
callbackDispatcher.wakeListenerServiceWithRequestStart(context, requestId);
6563

@@ -78,63 +76,79 @@ public UploadStatus processRequest(Context context, RequestParams params) {
7876

7977
ErrorInfo error = null;
8078

81-
if (optionsLoadedSuccessfully) {
82-
if (StringUtils.isNotBlank(uri)) {
83-
Payload payload = PayloadFactory.fromUri(uri);
84-
if (payload != null) {
85-
try {
86-
runningJobs.incrementAndGet();
87-
resultData = doProcess(requestId, appContext, options, params, payload);
88-
requestResultStatus = SUCCESS;
89-
} catch (FileNotFoundException e) {
90-
Logger.e(TAG, String.format("FileNotFoundException for request %s.", requestId), e);
91-
requestResultStatus = FAILURE;
92-
error = new ErrorInfo(ErrorInfo.FILE_DOES_NOT_EXIST, e.getMessage());
93-
} catch (LocalUriNotFoundException e) {
94-
Logger.e(TAG, String.format("LocalUriNotFoundException for request %s.", requestId), e);
95-
requestResultStatus = FAILURE;
96-
error = new ErrorInfo(ErrorInfo.URI_DOES_NOT_EXIST, e.getMessage());
97-
} catch (ResourceNotFoundException e) {
98-
Logger.e(TAG, String.format("ResourceNotFoundException for request %s.", requestId), e);
99-
error = new ErrorInfo(ErrorInfo.RESOURCE_DOES_NOT_EXIST, e.getMessage());
100-
requestResultStatus = FAILURE;
101-
} catch (EmptyByteArrayException e) {
102-
Logger.e(TAG, String.format("EmptyByteArrayException for request %s.", requestId), e);
103-
requestResultStatus = FAILURE;
104-
error = new ErrorInfo(ErrorInfo.BYTE_ARRAY_PAYLOAD_EMPTY, e.getMessage());
105-
} catch (InterruptedIOException e) {
106-
Logger.e(TAG, String.format("InterruptedIO exception for request %s.", requestId), e);
107-
error = new ErrorInfo(ErrorInfo.REQUEST_CANCELLED, "Request cancelled.");
108-
requestResultStatus = FAILURE;
109-
} catch (ErrorRetrievingSignatureException e) {
110-
Logger.e(TAG, String.format("Error retrieving signature for request %s.", requestId), e);
111-
requestResultStatus = FAILURE;
112-
error = new ErrorInfo(ErrorInfo.SIGNATURE_FAILURE, e.getMessage());
113-
} catch (IOException e) {
114-
Logger.e(TAG, String.format("IOException for request %s.", requestId), e);
115-
error = new ErrorInfo(ErrorInfo.NETWORK_ERROR, e.getMessage());
116-
requestResultStatus = RESCHEDULE;
117-
} catch (Exception e) {
118-
Logger.e(TAG, String.format("Unexpected exception for request %s.", requestId), e);
119-
error = new ErrorInfo(ErrorInfo.UNKNOWN_ERROR, e.getMessage());
79+
if (errorCount < maxErrorRetries) {
80+
if (optionsLoadedSuccessfully) {
81+
if (StringUtils.isNotBlank(uri)) {
82+
Payload payload = PayloadFactory.fromUri(uri);
83+
if (payload != null) {
84+
try {
85+
runningJobs.incrementAndGet();
86+
resultData = doProcess(requestId, appContext, options, params, payload);
87+
requestResultStatus = SUCCESS;
88+
} catch (FileNotFoundException e) {
89+
Logger.e(TAG, String.format("FileNotFoundException for request %s.", requestId), e);
90+
requestResultStatus = FAILURE;
91+
error = new ErrorInfo(ErrorInfo.FILE_DOES_NOT_EXIST, e.getMessage());
92+
} catch (LocalUriNotFoundException e) {
93+
Logger.e(TAG, String.format("LocalUriNotFoundException for request %s.", requestId), e);
94+
requestResultStatus = FAILURE;
95+
error = new ErrorInfo(ErrorInfo.URI_DOES_NOT_EXIST, e.getMessage());
96+
} catch (ResourceNotFoundException e) {
97+
Logger.e(TAG, String.format("ResourceNotFoundException for request %s.", requestId), e);
98+
error = new ErrorInfo(ErrorInfo.RESOURCE_DOES_NOT_EXIST, e.getMessage());
99+
requestResultStatus = FAILURE;
100+
} catch (EmptyByteArrayException e) {
101+
Logger.e(TAG, String.format("EmptyByteArrayException for request %s.", requestId), e);
102+
requestResultStatus = FAILURE;
103+
error = new ErrorInfo(ErrorInfo.BYTE_ARRAY_PAYLOAD_EMPTY, e.getMessage());
104+
} catch (InterruptedIOException e) {
105+
Logger.e(TAG, String.format("InterruptedIO exception for request %s.", requestId), e);
106+
error = new ErrorInfo(ErrorInfo.REQUEST_CANCELLED, "Request cancelled.");
107+
requestResultStatus = FAILURE;
108+
} catch (ErrorRetrievingSignatureException e) {
109+
Logger.e(TAG, String.format("Error retrieving signature for request %s.", requestId), e);
110+
requestResultStatus = FAILURE;
111+
error = new ErrorInfo(ErrorInfo.SIGNATURE_FAILURE, e.getMessage());
112+
} catch (IOException e) {
113+
Logger.e(TAG, String.format("IOException for request %s.", requestId), e);
114+
115+
if (errorCount >= maxErrorRetries) {
116+
// failure
117+
error = getMaxRetryError(errorCount);
118+
requestResultStatus = FAILURE;
119+
} else {
120+
// one up error count and reschedule
121+
params.putInt(ERROR_COUNT_PARAM, errorCount + 1);
122+
error = new ErrorInfo(ErrorInfo.NETWORK_ERROR, e.getMessage());
123+
requestResultStatus = RESCHEDULE;
124+
}
125+
} catch (Exception e) {
126+
Logger.e(TAG, String.format("Unexpected exception for request %s.", requestId), e);
127+
error = new ErrorInfo(ErrorInfo.UNKNOWN_ERROR, e.getMessage());
128+
requestResultStatus = FAILURE;
129+
} finally {
130+
runningJobs.decrementAndGet();
131+
}
132+
} else {
133+
Logger.d(TAG, String.format("Failing request %s, payload cannot be loaded.", requestId));
134+
error = new ErrorInfo(ErrorInfo.PAYLOAD_LOAD_FAILURE, "Request payload could not be loaded.");
120135
requestResultStatus = FAILURE;
121-
} finally {
122-
runningJobs.decrementAndGet();
123136
}
124137
} else {
125-
Logger.d(TAG, String.format("Failing request %s, payload cannot be loaded.", requestId));
126-
error = new ErrorInfo(ErrorInfo.PAYLOAD_LOAD_FAILURE, "Request payload could not be loaded.");
127138
requestResultStatus = FAILURE;
139+
error = new ErrorInfo(ErrorInfo.PAYLOAD_EMPTY, "Request payload is empty.");
140+
Logger.d(TAG, String.format("Failing request %s, Uri is empty.", requestId));
128141
}
129142
} else {
130143
requestResultStatus = FAILURE;
131-
error = new ErrorInfo(ErrorInfo.PAYLOAD_EMPTY, "Request payload is empty.");
132-
Logger.d(TAG, String.format("Failing request %s, Uri is empty.", requestId));
144+
error = new ErrorInfo(ErrorInfo.OPTIONS_FAILURE, "Options could not be loaded.");
145+
Logger.d(TAG, String.format("Failing request %s, cannot load options.", requestId));
133146
}
134147
} else {
148+
// safety - this should not really happen - the last error should already trigger
149+
// as failure and should not be rescheduled
150+
error = getMaxRetryError(errorCount);
135151
requestResultStatus = FAILURE;
136-
error = new ErrorInfo(ErrorInfo.OPTIONS_FAILURE, "Options could not be loaded.");
137-
Logger.d(TAG, String.format("Failing request %s, cannot load options.", requestId));
138152
}
139153

140154
if (requestResultStatus.isFinal()) {
@@ -147,7 +161,6 @@ public UploadStatus processRequest(Context context, RequestParams params) {
147161
// wake up the listener (this is not needed for reschedules since if the listener is down no point in reschedule notification.
148162
// we'll wake them up once the request is actually finished.
149163
callbackDispatcher.wakeListenerServiceWithRequestFinished(context, requestId, requestResultStatus);
150-
151164
} else {
152165
callbackDispatcher.dispatchReschedule(context, requestId, error);
153166
}
@@ -157,7 +170,19 @@ public UploadStatus processRequest(Context context, RequestParams params) {
157170
return requestResultStatus;
158171
}
159172

160-
private Map doProcess(final String requestId, Context appContext, Map<String, Object> options, RequestParams params, Payload payload) throws PayloadNotFoundException, IOException, ErrorRetrievingSignatureException {
173+
@NonNull
174+
private ErrorInfo getMaxRetryError(int errorCount) {
175+
ErrorInfo error;
176+
String message = String.format(Locale.getDefault(),
177+
"Request reached max retries allowed (%d).",
178+
errorCount);
179+
error = new ErrorInfo(ErrorInfo.TOO_MANY_ERRORS, message);
180+
return error;
181+
}
182+
183+
private Map doProcess(final String requestId, Context
184+
appContext, Map<String, Object> options, RequestParams params, Payload payload) throws
185+
PayloadNotFoundException, IOException, ErrorRetrievingSignatureException {
161186
Logger.d(TAG, String.format("Starting upload for request %s", requestId));
162187
Object preparedPayload = payload.prepare(appContext);
163188
final long actualTotalBytes = payload.getLength(appContext);

‎lib/src/main/java/com/cloudinary/android/callback/ErrorInfo.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public class ErrorInfo {
1717
public static final int BYTE_ARRAY_PAYLOAD_EMPTY = 10;
1818
public static final int REQUEST_CANCELLED = 11;
1919
public static final int PREPROCESS_ERROR = 12;
20+
public static final int TOO_MANY_ERRORS = 13;
2021

2122
private final int code;
2223
private final String description;

‎sample/src/main/java/com/cloudinary/android/sample/core/CloudinaryHelper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static String uploadResource(Resource resource, boolean preprocess) {
2626
.constrain(TimeWindow.getDefault())
2727
.option("resource_type", "auto")
2828
.maxFileSize(100 * 1024 * 1024) // max 100mb
29-
.policy(MediaManager.get().getGlobalUploadPolicy().newBuilder().maxRetries(10).build());
29+
.policy(MediaManager.get().getGlobalUploadPolicy().newBuilder().maxRetries(2).build());
3030
if (preprocess) {
3131
// scale down images above 2000 width/height, and re-encode as webp with 80 quality to save bandwidth
3232
request.preprocess(ImagePreprocessChain.limitDimensionsChain(2000, 2000)

0 commit comments

Comments
 (0)