Skip to content

Conversation

@pranavrajgopal
Copy link
Contributor

Adding API error handling and better error messages

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 16, 2021

Coverage Report

Affected SDKs

  • firebase-app-distribution

    SDK overall coverage changed from 61.92% (52e1cf8) to 64.57% (9892f00f) by +2.65%.

    Filename Base (52e1cf8) Head (9892f00f) Diff
    Constants.java ? 0.00% ?
    FirebaseAppDistribution.java 66.84% 71.58% +4.74%
    FirebaseAppDistributionException.java 58.82% 70.00% +11.18%
    FirebaseAppDistributionTesterApiClient.java 80.36% 80.95% +0.60%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (9892f00f) is created by Prow via merging commits: 52e1cf8 21c901d.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 16, 2021

Binary Size Report

Affected SDKs

  • firebase-app-distribution

    Type Base (52e1cf8) Head (9892f00f) Diff
    aar 48.5 kB 50.0 kB +1.50 kB (+3.1%)

Test Logs

Notes

Head commit (9892f00f) is created by Prow via merging commits: 52e1cf8 21c901d.


@NonNull private final Status status;
@Nullable private final AppDistributionRelease release;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing these because we should always pass a message to the FirebaseAppDistributionException since its customer facing.

setSignInTaskCompletionError(
new FirebaseAppDistributionException(AUTHENTICATION_CANCELED));
new FirebaseAppDistributionException(
"Tester cancelled authentication flow", AUTHENTICATION_CANCELED));

Choose a reason for hiding this comment

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

Should this string also be in the constants class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Fixed it

@pranavrajgopal
Copy link
Contributor Author

/retest

@pranavrajgopal pranavrajgopal force-pushed the fad-http-error-handling branch from 9c4ee05 to df97751 Compare July 19, 2021 18:16
@pranavrajgopal pranavrajgopal merged commit 4270d62 into master Jul 19, 2021
@pranavrajgopal pranavrajgopal deleted the fad-http-error-handling branch July 19, 2021 19:59
@firebase firebase locked and limited conversation to collaborators Aug 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes Override cla size/L

4 participants