-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose generic error codes for CredentialsManagerError #1076
Conversation
src/utils/baseError.ts
Outdated
@@ -1,12 +1,14 @@ | |||
class BaseError extends Error { | |||
constructor(name: string, message: string) { | |||
generic_error_code: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use field name like
identifier
reason
type
Or something that will be generic and can fit in a single word.
The purpose of the field will be extensively documented so it would be a one time learning that it is a platform agnostic error code.
import { User } from '../types'; | ||
import { deepEqual } from '../utils/deepEqual'; | ||
import { AuthState } from './auth0-context'; | ||
|
||
type Action = | ||
| { type: 'LOGIN_COMPLETE'; user: User } | ||
| { type: 'LOGOUT_COMPLETE' } | ||
| { type: 'ERROR'; error: Error } | ||
| { type: 'ERROR'; error: BaseError } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What value do we provide to generic_error_code
if for authentication errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is left empty.
The field is currently populated only in case of a CredentialManagerException.
We can re-use this field when we collate the Authentication Errors as well (tracked as part of SDK-5700).
@@ -30,6 +30,43 @@ | |||
|
|||
public class A0Auth0Module extends ReactContextBaseJavaModule implements ActivityEventListener { | |||
|
|||
private final Map<CredentialsManagerException, String> ERROR_CODE_MAP = new HashMap<>() {{ | |||
put(CredentialsManagerException.Companion.getINVALID_CREDENTIALS(), "INVALID_CREDENTIALS"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is Enum.name
field provided by Enums in Kotlin, can we use that to simplify the code? I haven't fired up my IDE to test this but looks like there is an opportunity to clean this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Enums are internal in the underlying android SDK right now and we cannot access them.
We are leveraging the Public fields exposed via the Companion class.
However, we have a plan to migrate this .java code to .kt. Once that is done, we can leverage kotlin semantics and clean this snippet (tracked as part of SDK-5699)
f07d761
to
4e14799
Compare
Changes
Added
We now expose platform agnostic generic error codes that represent different exception scenarios common in both android and iOS.
We have considered only the
CredentialsManagerError
for now.Keeping the backward compatibility in mind, a new field with the generic error code has been exposed via the
BaseError
class.The users can leverage the field as below :
Note : The old behaviour still remains untouched.
References
Testing
Checklist