-
Notifications
You must be signed in to change notification settings - Fork 617
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
Changes from 3 commits
7e56b11
ea217f6
d7233bc
2171113
3883cd0
fb9dabc
881a583
10245ae
12124fa
be97c04
a8c14d3
b8af30f
550d711
1d7cadf
435f8c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,15 +27,16 @@ public GetIdListener(TaskCompletionSource<String> taskCompletionSource) { | |
@Override | ||
public boolean onStateReached(PersistedFidEntry persistedFidEntry) { | ||
if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { | ||
taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); | ||
taskCompletionSource.trySetResult(persistedFidEntry.getFirebaseInstallationId()); | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
@Override | ||
public boolean onException(PersistedFidEntry persistedFidEntry, Exception exception) { | ||
if (persistedFidEntry.isErrored()) { | ||
taskCompletionSource.setException( | ||
new FirebaseInstallationsException( | ||
"Failed to update client side cache.", | ||
FirebaseInstallationsException.Status.CLIENT_ERROR)); | ||
taskCompletionSource.trySetException(exception); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, sad! I thought we had moved past retrolambda |
||
return true; | ||
} | ||
return false; | ||
|
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.
when would it be the case that we'd get an exception without the persistedFidEntry being in the error state?
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.
Interrupted Exception
Exception thrown while persisting.
Thread failure