sumup / sumup-ios-sdk

Other
46 stars 24 forks source link

`checkoutWithRequest:fromViewController:completion:` should use NSError default keys to provide concise error info #11

Open alvaromb opened 8 years ago

alvaromb commented 8 years ago

As per https://developer.apple.com/library/ios/documentation/Cocoa/Reference/Foundation/Classes/NSError_Class/index.html#//apple_ref/occ/cl/NSError, NSError has a default way to handle application errors with localized description keys. However, when performing a checkout with checkoutWithRequest:fromViewController:completion: that causes an error, localizedFailureReason, localizedRecoverySuggestion and localizedRecoveryOptions return nil.

Inside the error's userInfo there is useful information, this is the console output of a po [error userInfo] lldb command.

screen shot 2016-04-04 at 12 14 42

It's a bit tricky to access to the message key, which contains relevant localized information about the error.

screen shot 2016-04-04 at 12 17 32

Populating the localized keys with the info of the underlying error would be very helpful.

mollidor commented 8 years ago

Hi @alvaromb

Good point. These messages are displayed by the SumUp app indeed. We've thus added the error per the NSUnderlyingErrorKey, which is the "recommended standard way to embed NSErrors from underlying calls". However we missed to populate the localized.... properties of the error in a standard way in the SDK.

We will try to add them in a future SDK version, most likely as NSLocalizedDescriptionKey.

Regards, Lukas

alvaromb commented 8 years ago

Yes, I got the underlying error using that key. However, depending on your debug schema (or Xcode bugs), sometimes the underlying error is unaccessible through lldb and NSError's NSUnderlyingErrorKey.

For future reference, I got the message using the following code:

NSString *errorMessageKeyPath = [NSString stringWithFormat:@"%@.userInfo.JSONObject.data.message", NSUnderlyingErrorKey];
NSString *message = [[error userInfo] valueForKeyPath:errorMessageKeyPath];
larssn commented 6 years ago

Can we revisit this issue? Is it still on the table? It's been two years.

mollidor commented 6 years ago

Hi @larssn ,

thanks for bringing this up again and sorry for dropping the ball on this issue. We'll revisit it and ping you once we have an update.

Regards, Lukas