Skip to content

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

Merged
merged 33 commits into from
Aug 18, 2020
Merged

Conversation

hiranya911
Copy link
Contributor

@hiranya911 hiranya911 commented Aug 3, 2020

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 an AuthErrorCode value.
RELEASE NOTE: change(auth): All implementations of the UserImportHash abstract class are now marked final.

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 a MessagingErrorCode value.
RELEASE NOTE: change(fcm): Public constructors of the Notification class have been removed. Use the Notification.builder() to create new instances.

Resolves #354

hiranya911 and others added 25 commits January 24, 2020 13:10
* 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
@hiranya911 hiranya911 added the release:stage Stage a release candidate label Aug 3, 2020
@hiranya911
Copy link
Contributor Author

@egilmorez can you take a look at the API doc portions in the following classes?

  • ErrorCode
  • AuthErrorCode
  • MessagingErrorCode
  • FirebaseException
  • FirebaseAuthException
  • FirebaseMessagingException
  • IncomingHttpResponse
  • OutgoingHttpRequest

@hiranya911
Copy link
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.

CONFLICT,

/**
* Concurrency conflict, such as read-modify-write conflict.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional dupe?

Copy link
Contributor Author

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.

Copy link
Contributor

@egilmorez egilmorez left a 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.
Copy link
Contributor

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" ?

Copy link
Contributor Author

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..."

Comment on lines 39 to 40
/**
* Creates an OutgoingHttpRequest from the HTTP method and URL.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Creates an OutgoingHttpRequest from the HTTP method and URL.
/**
* Creates an `OutgoingHttpRequest` from the HTTP method and URL.

Copy link
Contributor Author

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.

Co-authored-by: egilmorez <[email protected]>
Co-authored-by: Kevin Cheung <[email protected]>
@hiranya911 hiranya911 removed the release:stage Stage a release candidate label Aug 14, 2020
@hiranya911
Copy link
Contributor Author

Thanks @egilmorez. I accepted most of the suggestions as is. For code-formatting type names we have to use the {@code} tag in Java, which I've applied manually.

Copy link
Contributor

@egilmorez egilmorez left a 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!

@hiranya911 hiranya911 assigned hiranya911 and unassigned egilmorez Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error handling refactor
4 participants