change: New error handling APIs (merging v7 branch into master) - #465
Merged
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
Contributor
Author
|
@egilmorez can you take a look at the API doc portions in the following classes?
|
Contributor
Author
|
@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. |
kevinthecheung
approved these changes
Aug 13, 2020
| CONFLICT, | ||
|
|
||
| /** | ||
| * Concurrency conflict, such as read-modify-write conflict. |
Contributor
Author
There was a problem hiding this comment.
Yes. These 2 error codes are identical in semantics. We should be able to fold them into one in a future major release.
egilmorez
reviewed
Aug 13, 2020
egilmorez
left a comment
Contributor
There was a problem hiding this comment.
Made a few style suggestions, thanks!
| /** | ||
| * Creates an IncomingHttpResponse from an HTTP error response. | ||
| * | ||
| * @param e HttpResponseException representing the HTTP error response. |
Contributor
There was a problem hiding this comment.
In cases like this, do you think it's best to say "HttpResponseException object" ?
Contributor
Author
There was a problem hiding this comment.
Changed to "Exception representing the..."
Comment on lines
+39
to
+40
| /** | ||
| * Creates an OutgoingHttpRequest from the HTTP method and URL. |
Contributor
There was a problem hiding this comment.
Suggested change
| /** | |
| * Creates an OutgoingHttpRequest from the HTTP method and URL. | |
| /** | |
| * Creates an `OutgoingHttpRequest` from the HTTP method and URL. |
Contributor
Author
There was a problem hiding this comment.
Java requires {@code } syntax. Fixed manually.
Co-authored-by: egilmorez <egilmore@google.com> Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Contributor
Author
|
Thanks @egilmorez. I accepted most of the suggestions as is. For code-formatting type names we have to use the |
egilmorez
approved these changes
Aug 17, 2020
egilmorez
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the updates Hiranya!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
ErrorCodeenum that defines all platform error codes.RELEASE NOTE: feat: Added a new
IncomingHttpResponseclass that can be used to access the HTTP response object associated with an exception.RELEASE NOTE: feat(auth): Added a new
AuthErrorCodeenum that defines all Firebase Auth error codes.RELEASE NOTE: change(auth):
FirebaseAuthExceptionnow exposes its error code as anAuthErrorCodevalue.RELEASE NOTE: change(auth): All implementations of the
UserImportHashabstract class are now markedfinal.RELEASE NOTE: feat(fcm): Added a new
MessagingErrorCodeenum that defines all Firebase Cloud Messaging error codes.RELEASE NOTE: change(fcm):
FirebaseMessagingExceptionnow exposes its error code as aMessagingErrorCodevalue.RELEASE NOTE: change(fcm): Public constructors of the
Notificationclass have been removed. Use theNotification.builder()to create new instances.Resolves #354