Skip to content

Propagating the exceptions to the clients. #856

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 15 commits into from
Oct 10, 2019
Merged

Conversation

ankitaj224
Copy link
Contributor

No description provided.

@ankitaj224 ankitaj224 requested a review from ciarand September 27, 2019 18:15
@googlebot googlebot added the cla: yes Override cla label Sep 27, 2019
@ankitaj224 ankitaj224 self-assigned this Sep 27, 2019

@Override
public boolean onException(PersistedFidEntry persistedFidEntry, Exception exception) {
if (persistedFidEntry.isErrored()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

when would it be the case that we'd get an exception without the persistedFidEntry being in the error state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interrupted Exception
Exception thrown while persisting.
Thread failure

new FirebaseInstallationsException(
"Failed to update client side cache.",
FirebaseInstallationsException.Status.CLIENT_ERROR));
taskCompletionSource.trySetException(exception);
Copy link
Contributor

Choose a reason for hiding this comment

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

given this is the same in each of the listener impls so far we could move it into a default interface method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error: [NoStaticOrDefaultMethodsInInterfaces] Avoid static/default methods in interfaces: We currently desugar SDKs with retrolambda which has limited support for those.

Default methods require API level 24. Current min is 14

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, sad! I thought we had moved past retrolambda

@ankitaj224 ankitaj224 requested a review from ciarand September 30, 2019 21:01
@ankitaj224
Copy link
Contributor Author

/test check-changed

new FirebaseInstallationsException(
"Failed to update client side cache.",
FirebaseInstallationsException.Status.CLIENT_ERROR));
taskCompletionSource.trySetException(exception);
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, sad! I thought we had moved past retrolambda

@ankitaj224
Copy link
Contributor Author

/test check-changed

@ankitaj224
Copy link
Contributor Author

/test api-information

@ankitaj224
Copy link
Contributor Author

/test new-smoke-tests

@ankitaj224
Copy link
Contributor Author

/retest

@ankitaj224
Copy link
Contributor Author

/test check-changed

@ankitaj224
Copy link
Contributor Author

/retest

@ankitaj224 ankitaj224 merged commit 56d1eab into listeners Oct 10, 2019
@ankitaj224 ankitaj224 deleted the fis-exceptions branch October 10, 2019 20:14
ankitaj224 added a commit that referenced this pull request Oct 10, 2019
* Simplifying FirebaseInstallations class by adding listeners.

* Addressing ciaran's comments to return same token if multiple getAuthToken()
calls are triggered simultaneously.

* Cleaning doRegistration method.

* Fixing FISClient to correctly parse expiration timestamp. (#848)

* Updating getAuthToken to return creation timestamp (#884)

* Propagating the exceptions to the clients. (#856)
ankitaj224 added a commit that referenced this pull request Oct 14, 2019
* Implemneting retry for FIS Client.

* Simplifying FirebaseInstallations class by adding listeners. (#847)

* Simplifying FirebaseInstallations class by adding listeners.

* Addressing ciaran's comments to return same token if multiple getAuthToken()
calls are triggered simultaneously.

* Cleaning doRegistration method.

* Fixing FISClient to correctly parse expiration timestamp. (#848)

* Updating getAuthToken to return creation timestamp (#884)

* Propagating the exceptions to the clients. (#856)

* Addressing ciaran's comments.

* Merge conflicts.

* Nit fixes

* Fixing verify format issue.
ankitaj224 added a commit that referenced this pull request Oct 14, 2019
* Fixing FISClient to correctly parse expiration timestamp. (#848)

* Updating getAuthToken to return creation timestamp (#884)

* Propagating the exceptions to the clients. (#856)

* Extract FID from FIS createInstallation response (#888)

* Addressing Rayo's comment: Rename PersistedFidEntry to (#899)

* Implementing retry once for FIS Client. (#895)

* Simplifying FirebaseInstallations class by adding listeners. (#847)

* Fixing FISClient to correctly parse expiration timestamp. (#848)

* Updating getAuthToken to return creation timestamp (#884)

* Propagating the exceptions to the clients. (#856)
@firebase firebase locked and limited conversation to collaborators Nov 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Override cla size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants