-
Notifications
You must be signed in to change notification settings - Fork 289
change: New error handling APIs (merging v7 branch into master) #465
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
Conversation
* Added core types for the error handling revamp * Fixed copyright year
* Added core error handling abstractions * Added unit tests for the new functionality * Exposed getErrorCode as getErrorCodeNew * Enabled error code assertions
* Added core error handling abstractions * Added unit tests for the new functionality * Exposed getErrorCode as getErrorCodeNew * Enabled error code assertions * Error handling revamp for FCM APIs * Cleaned up the FirebaseMessagingException * Cleaned up the AbstractHttpErrorHandler class * Updated tests
* Delete instance ID API error handling revamp * Added tests for IO and parse errors
…heme (#360) * Error handling revamp in FirebaseUserManager * Updated integration tests; Added documentation * Assigning the correct ErrorCode for auth errors * Moved AuthErrorHandler to a separate top-level class
* Error handling revamp for token verification APIs * Updated javadocs
* Error handlign revamp for the custom token creation API * Using the correct authorized HTTP client for IAM requests
* Error handling revamp for the project management API * Minor code and test cleanup * Fixed some lint errors; Removed requestFactory reference from project mgt service impl
* Minor code and test cleanup * Renamed getErrorCodeNew() to getErrorCode() in FirebaseException * Fixing checkstyle error
* fix: Removed unused FirebaseAppStore abstraction * Using the keySet of App instances to populate the app names list
…ngHttpClient (#462) * fix: Handling JSON serialization and response interception at ErrorHandlingHttpClient * fix: Removing redundant method override header
* feat: Added new error codes for IdP management and multitenancy * fix: Updated integration tests * fix: Renamed helper method
* chore: Support for specifying query parameters in HttpRequestInfo * fix: Removing redundant JsonObjectParser from HttpClient
@egilmorez can you take a look at the API doc portions in the following classes?
|
@kevinthecheung adding you to this as well since you're familiar with the Auth API surface. Appreciate if you can review the API docs for the classes listed in my previous comment. |
CONFLICT, | ||
|
||
/** | ||
* Concurrency conflict, such as read-modify-write conflict. |
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.
Intentional dupe?
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.
Yes. These 2 error codes are identical in semantics. We should be able to fold them into one in a future major release.
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.
Made a few style suggestions, thanks!
/** | ||
* Creates an IncomingHttpResponse from an HTTP error response. | ||
* | ||
* @param e HttpResponseException representing the HTTP error response. |
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.
In cases like this, do you think it's best to say "HttpResponseException
object" ?
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.
Changed to "Exception representing the..."
/** | ||
* Creates an OutgoingHttpRequest from the HTTP method and URL. |
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.
/** | |
* Creates an OutgoingHttpRequest from the HTTP method and URL. | |
/** | |
* Creates an `OutgoingHttpRequest` from the HTTP method and URL. |
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.
Java requires {@code }
syntax. Fixed manually.
src/main/java/com/google/firebase/messaging/MessagingErrorCode.java
Outdated
Show resolved
Hide resolved
Co-authored-by: egilmorez <[email protected]> Co-authored-by: Kevin Cheung <[email protected]>
Thanks @egilmorez. I accepted most of the suggestions as is. For code-formatting type names we have to use the |
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.
Thanks for the updates Hiranya!
RELEASE NOTE: feat: Added a collection of new APIs for implementing advanced error handling logic. See error handling guide for more details.
RELEASE NOTE: feat: Added a new
ErrorCode
enum that defines all platform error codes.RELEASE NOTE: feat: Added a new
IncomingHttpResponse
class that can be used to access the HTTP response object associated with an exception.RELEASE NOTE: feat(auth): Added a new
AuthErrorCode
enum that defines all Firebase Auth error codes.RELEASE NOTE: change(auth):
FirebaseAuthException
now exposes its error code as anAuthErrorCode
value.RELEASE NOTE: change(auth): All implementations of the
UserImportHash
abstract class are now markedfinal
.RELEASE NOTE: feat(fcm): Added a new
MessagingErrorCode
enum that defines all Firebase Cloud Messaging error codes.RELEASE NOTE: change(fcm):
FirebaseMessagingException
now exposes its error code as aMessagingErrorCode
value.RELEASE NOTE: change(fcm): Public constructors of the
Notification
class have been removed. Use theNotification.builder()
to create new instances.Resolves #354