Use ClientException instead of try catch block in sample app #24
Conversation
….message ?: error.errorMessage instead of error.toString() when displaying dialog.
### Changes summary: 1. Add GetAccountError and SignOutError error in Error.kt 2. Use try catch block with corresponding errors in interface methods. 3. Add testEmptyRequestParametersToGenericErrorNotThrownException() ### Company PRs: native sample app: Azure-Samples/ms-identity-ciam-native-auth-android-sample#24
...main/java/com/azuresamples/msalnativeauthandroidkotlinsampleapp/EmailSignInSignUpFragment.kt
Outdated
Show resolved
Hide resolved
| val password = CharArray(binding.passwordText.length()) | ||
| binding.passwordText.text?.getChars(0, binding.passwordText.length(), password, 0) | ||
|
|
||
| val actionResult: SignInResult | ||
| try { |
There was a problem hiding this comment.
Why do we still need this try? Also, a try without a catch?
There was a problem hiding this comment.
Applies to multiple places.
There was a problem hiding this comment.
I think it's because there is a try-finally block which has relation with the password security/privacy fix completed by Burak before.
There was a problem hiding this comment.
I get the point of the try-finally now, but I don't think we need a try at all now anymore. The SDK shouldn't throw an exception, so developers don't need to expect it.
There was a problem hiding this comment.
Removed try finally block
| @@ -80,47 +81,46 @@ class AccessApiFragment : Fragment() { | |||
| is GetAccountResult.NoAccountFound -> { | |||
| displaySignedOutState() | |||
| } | |||
| is GetAccountError -> { | |||
| displayDialog(getString(R.string.msal_exception_title), accountResult.exception?.message ?: accountResult.errorMessage) | |||
There was a problem hiding this comment.
For simplicity, let's just use accountResult.errorMessage here. That's the ultimate error. For more details on the error, developers can look into the exception's message. What do you think?
If you agree, this comment applies to multiple places.
There was a problem hiding this comment.
Agreed. Will make the update
Yuki-YuXin
added
the
5.3.1>
# Conflicts: # app/src/main/java/com/azuresamples/msalnativeauthandroidkotlinsampleapp/AccessApiFragment.kt
Change summary:
Company PRs:
msal: AzureAD/microsoft-authentication-library-for-android#2080