[Documentation] Clarity around errors

[guidedways]

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:

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:

   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.

[greg-db]

Thanks for writing up this detailed feedback! I'm sending this along to the team to see if we can get some better and clearer documentation/guarantees/examples around this.

[guidedways]

Thanks Greg. I should add, ideally it would have been great if there was simply one error returned, irrespective of whether it's was an API error or a Network failure. Having two separate errors and not knowing which one takes precedence over the other is confusing, at least without further clarity / examples etc. I realize batch upload / delete is still one of those edge cases where one of the files being uploaded could fail, and so the current mechanism of differentiating a failure from success works well, but then the other two errors should be guaranteed to always be nil (again - I wish the two errors could just been combined).

The fact that we're getting an 'API Error' back is already strange and unfriendly. Imagine if for every API call within Core Foundation on the mac, we were to receive a potential internal API error as well. Why should there be a need for one? That sounds like the developers of the API aren't confident their API isn't going to fail, or if it does fail, it should gracefully throw back a single exception / error for us to handle and then retry. In fact, I'd argue API errors should either be automatically retried OR if it's a fatal error (such as the local file provided does not exist), it should still be combined within a single error that's returned with a unique error code (error codes could be reserved internally 1000 onwards). That's just how the CFNetwork et al work. The error returned has an error code to differentiate the type of error it is (network vs. internal).

I realize that's not going to happen any time soon, so until then it'd be great to just have more covered in terms of documentation and examples :)