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
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7e56b11
Propagating the exceptions to the clients.
ankitaj224 Sep 27, 2019
ea217f6
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 27, 2019
d7233bc
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 27, 2019
2171113
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 29, 2019
3883cd0
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 29, 2019
fb9dabc
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 30, 2019
881a583
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Sep 30, 2019
10245ae
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 1, 2019
12124fa
Fixing formatting issue.
ankitaj224 Oct 8, 2019
be97c04
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 8, 2019
a8c14d3
Adding javadocs for StateListener class.
ankitaj224 Oct 8, 2019
b8af30f
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 8, 2019
550d711
Fixing format issue.
ankitaj224 Oct 9, 2019
1d7cadf
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 9, 2019
435f8c8
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ public void testGetAuthToken_PersistedFidError_failure() throws Exception {
.isInstanceOf(FirebaseInstallationsException.class);
assertWithMessage("Exception status doesn't match")
.that(((FirebaseInstallationsException) expected.getCause()).getStatus())
.isEqualTo(FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
.isEqualTo(FirebaseInstallationsException.Status.CLIENT_ERROR);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,19 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) {
}
}

private void triggerOnException(PersistedFidEntry persistedFidEntry, Exception exception) {
synchronized (lock) {
Iterator<StateListener> it = listeners.iterator();
while (it.hasNext()) {
StateListener l = it.next();
boolean doneListening = l.onException(persistedFidEntry, exception);
if (doneListening) {
it.remove();
}
}
}
}

private final Runnable doRegistration() {
return () -> {
try {
Expand Down Expand Up @@ -238,7 +251,7 @@ private final Runnable doRegistration() {
.setRegistrationStatus(RegistrationStatus.REGISTER_ERROR)
.build();
persistedFid.insertOrUpdatePersistedFidEntry(errorFidEntry);
triggerOnStateReached(errorFidEntry);
triggerOnException(errorFidEntry, e);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@ public boolean onStateReached(PersistedFidEntry persistedFidEntry) {
.build());
return true;
}
return false;
}

@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

resultTaskCompletionSource.setException(
new FirebaseInstallationsException(
"Firebase Installation is not registered.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR));
resultTaskCompletionSource.trySetException(exception);
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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

return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@

interface StateListener {
boolean onStateReached(PersistedFidEntry persistedFidEntry);

boolean onException(PersistedFidEntry persistedFidEntry, Exception exception);
}