dropbox / dropbox/dropbox-sdk-obj-c

[Documentation] Clarity around errors

Open
#223 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Objective-C
Stars
186
Forks
118
Avg merge
3m
Merged PRs (30d)
9

Description

I have to admit, I'm always nervous when trying to handle errors ever since the Dropbox API has changed in v2. For me, it's no longer clear (either in documentation, or examples) how to handle the errors properly.

For instance:

```objc
setResponseBlock: ^(DBFILESFileMetadata *fileData, DBFILESUploadError *uploadError, DBRequestError *requestError) {
}
```

What happens if `requestError != nil`, will `uploadError` be guaranteed to be `nil`? And vice versa? Should I expect `fileData` to be *always* `nil` in case either of the two errors are non-nil?

Not to mention the various additional checks one has to perform in order to find the actual error deep inside of a `requestError` or `uploadError`.

I would truly appreciate if the examples showed more concrete examples of proper error handling. The only example on the main page regarding batchUploadFiles is a bit confusing:

```objc
if (fileUrlsToBatchResultEntries) {
NSLog(@"Call to `/upload_session/finish_batch/check` succeeded");
for (NSURL *clientSideFileUrl in fileUrlsToBatchResultEntries) {
DBFILESUploadSessionFinishBatchResultEntry *resultEntry = fileUrlsToBatchResultEntries[clientSideFileUrl];
if ([resultEntry isSuccess]) {
NSString *dropboxFilePath = resultEntry.success.pathDisplay;
NSLog(@"File successfully uploaded from %@ on local machine to %@ in Dropbox.",
[clientSideFileUrl path], dropboxFilePath);
} else if ([resultEntry isFailure]) {
// This particular file was not uploaded successfully, although the other
// files may have been uploaded successfully. Perhaps implement some retry
// logic here based on `uploadNetworkError` or `uploadSessionFinishError`
DBRequestError *uploadNetworkError = fileUrlsToRequestErrors[clientSideFileUrl];
DBFILESUploadSessionFinishError *uploadSessionFinishError = resultEntry.failure;

// implement appropriate retry logic
}
}
}

// Why is the following not inside of an } else { ?

if (finishBatchRouteError) {
NSLog(@"Either bug in SDK code, or transient error on Dropbox server");
NSLog(@"%@", finishBatchRouteError);
} else if (finishBatchRequestError) {
NSLog(@"Request error from calling `/upload_session/finish_batch/check`");
NSLog(@"%@", finishBatchRequestError);
} else if ([fileUrlsToRequestErrors count] > 0) {
NSLog(@"Other additional errors (e.g. file doesn't exist client-side, etc.).");
NSLog(@"%@", fileUrlsToRequestErrors);
}
```

This, to me, suggests that `fileUrlsToBatchResultEntries` could be non-nil *whilst* the errors also are there to be handled - and this makes me nervous, that I may not be handling my error cases correctly, or what do I do in case there is part success. What type and kind of an error do I consider for a 'retry' or when do I consider the call will always fail.

I guess the ability to inspect the `.tag` element is an elegant solution for the dozens of error types that could occur, it still requires us to 'hunt' for these manually each time. Each method call could have its own set of `tag` types and there's no assumption of uniformity. If only we had more examples demonstrating how to properly handle these errors and when to retry automatically. Frequently I've had Dropbox APIs fail on me for no reason (it's been like this for years) and I've had to retry with a sleep of 0.5 seconds up to 10 times at times and suddenly it begins to respond as you'd expect. I've never had to do this for any other server our app communicates with, which makes it more difficult to handle these errors efficiently within a wrapper method that attempts to retry.

I believe the Android SDK has a built-in retry mechanism within the SDK (if I recall correctly), which I thought was great! Right now I've had to choreograph a complicated wrapper method around the batch methods to retry failed attempts, whilst keeping track of successful uploads - and I'm still not a 100% sure if I've handled all the edge cases correctly.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.