Skip to content

Added core types for the error handling revamp #350

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 2 commits into from
Jan 24, 2020
Merged

Conversation

hiranya911
Copy link
Contributor

Starting to implement the error handling revamp. Since this contains sweeping breaking changes, I'm moving the implementation to a separate v7 branch.

  • Added ErrorCode, IncomingHttpResponse and OutgoingHttpRequest types.
  • Exposed error code and http response from the FirebaseException base class.
  • Deprecated the old constructors in FirebaseException. These will be removed once the error handling revamp is fully implemented.

go/firebase-error-handling-java

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM!
Just one small comment

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

Choose a reason for hiding this comment

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

Just to make sure, concurrency conflict and aborted are caused by the same reasons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CONFLICT is really an error code we made up. This is because HTTP 409 can be mapped to both ABORTED and ALREADY_EXISTS: https://cloud.google.com/apis/design/errors#handling_errors

To mitigate this ambiguity we introduced an error code CONFLICT, so that if we only have the HTTP status code to identify an error condition, we will map it to CONFLICT instead of one of the other two.

@hiranya911 hiranya911 merged commit b269191 into v7 Jan 24, 2020
@hiranya911 hiranya911 deleted the hkj-error-types branch January 24, 2020 21:10
hiranya911 added a commit that referenced this pull request Aug 18, 2020
* Added core types for the error handling revamp (#350)

* Added core types for the error handling revamp

* Fixed copyright year

* Added ErrorHandlingHttpClient API (#353)

* Added core error handling abstractions

* Added unit tests for the new functionality

* Exposed getErrorCode as getErrorCodeNew

* Enabled error code assertions

* Error handling revamp for the FirebaseMessaging API (#357)

* 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

* Error handling revamp for FirebaseInstanceId API (#359)

* Delete instance ID API error handling revamp

* Added tests for IO and parse errors

* fix(auth): Migrated user management APIs to the new error handling scheme (#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 (#362)

* Error handling revamp for token verification APIs

* Updated javadocs

* Error handling revamp for the custom token creation API (#366)

* 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 (#367)

* 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

* Renamed getErrorCodeNew() to getErrorCode() (#379)

* Minor code and test cleanup

* Renamed getErrorCodeNew() to getErrorCode() in FirebaseException

* Fixing checkstyle error

* Fixing some deprecation warnings (#380)

* Handling IID error codes correctly (#381)

* Removed old deprecated APIs (#383)

* fix: Removed unused FirebaseAppStore abstraction (#427)

* fix: Removed unused FirebaseAppStore abstraction

* Using the keySet of App instances to populate the app names list

* chore: Removing redundant test dependency (#441)

* chore: Make user import hashing classes final (#425)

* chore: Merged with v7 branch with master (#456)

* fix(fcm): Replacing deprecated Batch API constructor (#460)

* fix: Handling http method override in ErrorHandlingHttpClient (#459)

* fix: Handling JSON serialization/response interception at ErrorHandlingHttpClient (#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 (#458)

* feat: Added new error codes for IdP management and multitenancy

* fix: Updated integration tests

* fix: Renamed helper method

* fix: Removing some calls to deprecated APIs (#464)

* chore: Support for specifying query parameters in HttpRequestInfo (#463)

* chore: Support for specifying query parameters in HttpRequestInfo

* fix: Removing redundant JsonObjectParser from HttpClient

* fix: Fixing a series of javadoc warnings (#466)

* fix: Made some APIs on AbstractFirebaseAuth.Builder package-protected for consistency

* Apply suggestions from code review

Co-authored-by: egilmorez <egilmore@google.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>

* fix: Minor updates to API ref docs based on code review comments

* fix: Fixing API doc wording

Co-authored-by: Horatiu Lazu <horatiulazu@gmail.com>
Co-authored-by: egilmorez <egilmore@google.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants