Make a best effort attempt to flush reports at crash time #4112
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Coverage Report 1Affected Products
Test Logs |
Size Report 1Affected Products
Test Logs |
vkryachko
requested changes
Sep 16, 2022
...ase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/send/ReportQueue.java
Outdated
Show resolved
Hide resolved
Comment on lines
122
to
132
| new Thread( | ||
| () -> { | ||
| TransportRuntime.getInstance() | ||
| .getUploader() | ||
| .upload(DataTransportCrashlyticsReportSender.transportContext, 1, () -> {}); | ||
| latch.countDown(); | ||
| }) | ||
| .start(); | ||
| try { | ||
| //noinspection ResultOfMethodCal 10BC0 lIgnored best effort only | ||
| latch.await(2, TimeUnit.SECONDS); |
There was a problem hiding this comment.
If you block waiting for the thread to finish, why have a Thread at all instead of calling the blocking method directly?
There was a problem hiding this comment.
We want this to time out after 2 seconds, so it never leaves the app hanging after the uncaught exception.
|
@emilypgoogle jfyi |
vkryachko
reviewed
Sep 16, 2022
...java/com/google/firebase/crashlytics/internal/send/DataTransportCrashlyticsReportSender.java
Outdated
Show resolved
Hide resolved
vkryachko
reviewed
Sep 26, 2022
...t/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ForcedSender.java
Show resolved
Hide resolved
...t/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ForcedSender.java
Outdated
Show resolved
Hide resolved
...t/transport-runtime/src/main/java/com/google/android/datatransport/runtime/ForcedSender.java
Outdated
Show resolved
Hide resolved
vkryachko
reviewed
Sep 26, 2022
...rc/main/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/Uploader.java
Show resolved
Hide resolved
vkryachko
approved these changes
Sep 26, 2022
lfkellogg
added a commit
that referenced
this pull request
Oct 26, 2022
* Deflake firebase_common HeartBeat tests. (#4083) The tests relied on `TestOnCompleteListener` that was not safe to call more than once since it was based on a count down latch. So reusing it multiple times would cause await() to return immediately. This change makes it so that a new latch is created for every await() call, making all await() calls work. Fixes: http://b/245956774 * Add Javadoc support to the DackkaPlugin (#4082) * Add util method for copying directories * Add javadoc support to our dackka plugin * Remove the extension check on fromDirectory * Add a note about cache compliance and the javadoc task * Add reference to kotlin stdlib package list (#4093) * Add appcheck-ktx to bom config (#4081) * Removing getRunningAppProcesses since the process_name isn't used (#4057) * Fix Documentation classpath (#4099) b/241795594 * Add projectSpecificSources back to the DackkaPlugin (#4110) * Added extra method for TaskProviders * Added specificSources method back * Revert to dependsOn for docstubs dep * Revamp test harness for macrobenchmark tests (#4071) * Fix dependabot security alerts (#4123) * Make firesite transform cacheable (#4124) * add coroutines-play-services as a transitive dep to firebase-common-ktx (#4044) * add kotlinx-coroutines-play-services as a transitive dep to firebase-common-ktx * Update to Coroutines 1.6.4 * database-ktx: add callbackFlow for eventlisteners (#4012) * add callbackFlow for RTDB's ValueEventListener * add callbackFlow for RTDB's ChildEventListener * delegate trySendBlocking to DefaultRunLoop * add group to ktx.gradle * update api.txt file * Update released versions (#4135) * Upgrade dackkaConfig (#4141) * Add names to all Firebase components (#4117) * Add appcheck's ktx artifact back to package list file (#4142) * Add strict mode testing in firebase-messaging (#4095) * Add gralde property to instrument Fireperf E2E test (#4144) The perf gradle PR is #334 in the gradle repo. b/246802885 * Resolve StrictMode violation in App Check. (#4085) * Resolve StrictMode violation in App Check. * Attempt to fix some tests. * Fix unit tests. * Make `retrieveStoredAppCheckTokenInBackground` private instead of package-private. * Move listener invocations back to the main thread while keeping disk write on background thread. * Refactor to use lambda syntax. * Implemented exponential backoff and max retry with resumable uploads (#4087) * storage-ktx: add callbackFlow for upload/download progress (#4139) * add kotlin flows to storage * update api.txt file * add group to storage/ktx.gradle * Make a best effort attempt to flush reports at crash time (#4112) This should allow us to upload reports for start-up crashes. * Public Count (#4130) * Public Count * Disable prod testing * Long to long * Api.txt * Backfill changelog * Add PR * Fix assertEquals error * Re-write API javadocs for COUNT API (#4143) Co-authored-by: Denver Coneybeare <dconeybe@google.com> * [Fireperf][AASA] send `_experiment_app_start_ttid` trace, controlled by RC flag (#4114) * log _experiment_as_ttid * send event and RRC mitigation * add RC wip * modified save to cache when RC fetches * dev-app manifest override * unit test for RCc cache saving * better name and comments * better formatting remoteconfigmanagertest * better comments and added local RC lookup back * Specify unique ref tags in Dackka output (#4149) * Add util methods for gradle projects * Disable Javadoc generation on empty projects * Fix ref path generation in Dackka output * Add documentation for util methods * Update the DackkaPlugin docs * Reduced path to relative from tenant * Reduced ref head path even more * Fixed ref tag path to working solution * Disabled publishJavadoc by default for tests It should be enabled explicitly when being tested anyhow, and causes issues otherwise. Instead of disabling it for the tests that don't need it- this is much quicker and easier to manager. * Add strict mode tests to inappmessaging and inappmessaging-display (#4136) * Fix strict mode violations for appcheck (#4148) * Fix strict mode violations for appcheck * Formatting * Add copyright header * Populate SDKs changelog files (#4070) * first try seeding changelogs * Added unreleased section to CHANGELOG * Fix empty lines between sections * Add missing entries for abt * Update data to include latest releases * Update CHANGELOG.md * Add missing line in unreleased section for perf. * Enable CHANGELOG check globally (#4084) * Enable CHANGELOG check globally * Simplify conditional. * Enable COUNT integration tests, now that backend support has rolled out (#4163) * Remove separation of kotlin directories in dackka (#416 1E79 6) * Deprecate App Check SafetyNet SDK (#4187) * Add `@Deprecated` annotations to Firebase App Check SafetyNet SDK public API. * Add `@deprecated` tag in the Javadoc as well. * Remove stale entries from Unreleased section. (#4185) * Assign ConfigContainer Builder return values. (#4194) * update bom (#4155) * update bom * update * update * feat(perf-ktx): add trace(name, block) extension function (#4180) * Remove smoke test for app indexing (#4219) App Indexing is deprecated starting BoM 31.0.0 . https://firebase.google.com/support/release-notes/android#bom_v31-0-0 * Bump Robolectric to 4.9 (#4161) * Add plexus-utils for firebase-database tests Looks like firebase-database tests use plexus-utils dependency of Robolectric directly. But this dependency was removed by Robolectric. So this CL adds plexus-utils explictly for firebase-database tests. Signed-off-by: utzcoz <utzcoz@outlook.com> * Bump Robolectric to 4.9 1. Use legacy LooperMode for tests explicitly, because recent Robolectric releases switch to use PAUSED mode default. Before these tests migrate to PAUSED mode, they use LEGACY mode to pass tests. 2. Migrate Assert.assertThat to Truth.assertThat to avoid using removed APIs. 3. All build.gradle use the same robolectricVersion except transport-backend-cct because Robolectric 4.8+ has compatibility problem for TelephonyManager with low compile/targetSdkversion. To keep httpclient compatibility, transport-backend-ccts continues to use Robolectric 4.3.1. 4. Remove unused exclude protobuf-java from Robolectric. 5. Add necessary protobuf-lite dependency on classpath for some ktx modules' tests. Signed-off-by: utzcoz <utzcoz@outlook.com> Signed-off-by: utzcoz <utzcoz@outlook.com> * return exception if modelname is empty (#4226) * Add "create release PR" github action (#4236) This implementation: - Creates the base branch (name is based in user input) - Creates the release branch (name is based in user input) - Creates the release.cfg file in the release branch without adding any SDK (module) to it. It can create the branches based on any existing branch of the repo. * Sync spec tests from web SDK to Android SDK (#4230) * Update versions (#4238) * Update versions * Exclude .github dir from `firebaseContinuousIntegration` paths (#4239) * Performing IN expansion (#4221) * WIP: `in` expansion. * Add composite filter in-expansion test. * Fix formatting. * Run in-expansion as part of DNF computation and add tests. * Add test with nested IN filters with CSI. * Add tests for other cases. * typo fix (#4237) * Firestore: Add test that verifies count query error message when missing index (#4232) * refactor(functions): update firebase-iid to 21.1.0 (#4225) * refactor(functions): update firebase-iid to 21.1.0 * Update CHANGELOG.md * Update CHANGELOG.md * bump firebase-iid-interop to 17.1.0 * exclude firebase-components from firebase-iid dependency Signed-off-by: utzcoz <utzcoz@outlook.com> Co-authored-by: Vladimir Kryachko <vkryachko@google.com> Co-authored-by: Daymon <17409137+daymxn@users.noreply.github.com> Co-authored-by: Raymond Lam <raymondlam@google.com> Co-authored-by: Yifan Yang <yifayan@gmail.com> Co-authored-by: Rosário Pereira Fernandes <rosariopf@google.com> Co-authored-by: emilypgoogle <110422458+emilypgoogle@users.noreply.github.com> Co-authored-by: Jeremy Jiang <64182239+jeremyjiang-dev@users.noreply.github.com> Co-authored-by: Rosalyn Tan <rosalyntan@google.com> Co-authored-by: Maneesh Tewani <maneesht@users.noreply.github.com> Co-authored-by: Matthew Robertson <mrober@users.noreply.github.com> Co-authored-by: wu-hui <53845758+wu-hui@users.noreply.github.com> Co-authored-by: Denver Coneybeare <dconeybe@google.com> Co-authored-by: Leo Zhan <zhanl@google.com> Co-authored-by: Rodrigo Lazo <rlazo@users.noreply.github.com> Co-authored-by: Dana Silver <danasilver@google.com> Co-authored-by: Vinay Guthal <vguthal@google.com> Co-authored-by: utzcoz <43091780+utzcoz@users.noreply.github.com> Co-authored-by: argzdev <alvinrustan@google.com> Co-authored-by: Mila <107142260+milaGGL@users.noreply.github.com> Co-authored-by: Ehsan <ehsannas@gmail.com> Co-authored-by: cherylEnkidu <96084918+cherylEnkidu@users.noreply.github.com>
lfkellogg
added a commit
that referenced
this pull request
Oct 27, 2022
* Minor updates to the App Distribution test app. (#4088) * Update various versions (#4090) * Updated various versions (dependencies and SDK to fix Android resource linking failure (AAPT: error: resource android:attr/lStar not found.) * Update compileSdk/targetSdkVersion from 31 to 33 * Update with latest changes from master (#4091) * Deflake firebase_common HeartBeat tests. (#4083) The tests relied on `TestOnCompleteListener` that was not safe to call more than once since it was based on a count down latch. So reusing it multiple times would cause await() to return immediately. This change makes it so that a new latch is created for every await() call, making all await() calls work. Fixes: http://b/245956774 * Add Javadoc support to the DackkaPlugin (#4082) * Add util method for copying directories * Add javadoc support to our dackka plugin * Remove the extension check on fromDirectory * Add a note about cache compliance and the javadoc task * Add reference to kotlin stdlib package list (#4093) Co-authored-by: Vladimir Kryachko <vkryachko@google.com> Co-authored-by: Daymon <17409137+daymxn@users.noreply.github.com> * Resolve strict mode violations in firebase-appdistribution (#4092) * merge master into fad/next (#4102) * Deflake firebase_common HeartBeat tests. (#4083) The tests relied on `TestOnCompleteListener` that was not safe to call more than once since it was based on a count down latch. So reusing it multiple times would cause await() to return immediately. This change makes it so that a new latch is created for every await() call, making all await() calls work. Fixes: http://b/245956774 * Add Javadoc support to the DackkaPlugin (#4082) * Add util method for copying directories * Add javadoc support to our dackka plugin * Remove the extension check on fromDirectory * Add a note about cache compliance and the javadoc task * Add reference to kotlin stdlib package list (#4093) Co-authored-by: Vladimir Kryachko <vkryachko@google.com> Co-authored-by: Daymon <17409137+daymxn@users.noreply.github.com> * Remove `release` field. (#4174) * Merge changes from master into fad/next (#4199) * Merge latest master into fad/next (#4247) * Deflake firebase_common HeartBeat tests. (#4083) The tests relied on `TestOnCompleteListener` that was not safe to call more than once since it was based on a count down latch. So reusing it multiple times would cause await() to return immediately. This change makes it so that a new latch is created for every await() call, making all await() calls work. Fixes: http://b/245956774 * Add Javadoc support to the DackkaPlugin (#4082) * Add util method for copying directories * Add javadoc support to our dackka plugin * Remove the extension check on fromDirectory * Add a note about cache compliance and the javadoc task * Add reference to kotlin stdlib package list (#4093) * Add appcheck-ktx to bom config (#4081) * Removing getRunningAppProcesses since the process_name isn't used (#4057) * Fix Documentation classpath (#4099) b/241795594 * Add projectSpecificSources back to the DackkaPlugin (#4110) * Added extra method for TaskProviders * Added specificSources method back * Revert to dependsOn for docstubs dep * Revamp test harness for macrobenchmark tests (#4071) * Fix dependabot security alerts (#4123) * Make firesite transform cacheable (#4124) * add coroutines-play-services as a transitive dep to firebase-common-ktx (#4044) * add kotlinx-coroutines-play-services as a transitive dep to firebase-common-ktx * Update to Coroutines 1.6.4 * database-ktx: add callbackFlow for eventlisteners (#4012) * add callbackFlow for RTDB's ValueEventListener * add callbackFlow for RTDB's ChildEventListener * delegate trySendBlocking to DefaultRunLoop * add group to ktx.gradle * update api.txt file * Update released versions (#4135) * Upgrade dackkaConfig (#4141) * Add names to all Firebase components (#4117) * Add appcheck's ktx artifact back to package list file (#4142) * Add strict mode testing in firebase-messaging (#4095) * Add gralde property to instrument Fireperf E2E test (#4144) The perf gradle PR is #334 in the gradle repo. b/246802885 * Resolve StrictMode violation in App Check. (#4085) * Resolve StrictMode violation in App Check. * Attempt to fix some tests. * Fix unit tests. * Make `retrieveStoredAppCheckTokenInBackground` private instead of package-private. * Move listener invocations back to the main thread while keeping disk write on background thread. * Refactor to use lambda syntax. * Implemented exponential backoff and max retry with resumable uploads (#4087) * storage-ktx: add callbackFlow for upload/download progress (#4139) * add kotlin flows to storage * update api.txt file * add group to storage/ktx.gradle * Make a best effort attempt to flush reports at crash time (#4112) This should allow us to upload reports for start-up crashes. * Public Count (#4130) * Public Count * Disable prod testing * Long to long * Api.txt * Backfill changelog * Add PR * Fix assertEquals error * Re-write API javadocs for COUNT API (#4143) Co-authored-by: Denver Coneybeare <dconeybe@google.com> * [Fireperf][AASA] send `_experiment_app_start_ttid` trace, controlled by RC flag (#4114) * log _experiment_as_ttid * send event and RRC mitigation * add RC wip * modified save to cache when RC fetches * dev-app manifest override * unit test for RCc cache saving * better name and comments * better formatting remoteconfigmanagertest * better comments and added local RC lookup back * Specify unique ref tags in Dackka output (#4149) * Add util methods for gradle projects * Disable Javadoc generation on empty projects * Fix ref path generation in Dackka output * Add documentation for util methods * Update the DackkaPlugin docs * Reduced path to relative from tenant * Reduced ref head path even more * Fixed ref tag path to working solution * Disabled publishJavadoc by default for tests It should be enabled explicitly when being tested anyhow, and causes issues otherwise. Instead of disabling it for the tests that don't need it- this is much quicker and easier to manager. * Add strict mode tests to inappmessaging and inappmessaging-display (#4136) * Fix strict mode violations for appcheck (#4148) * Fix strict mode violations for appcheck * Formatting * Add copyright header * Populate SDKs changelog files (#4070) * first try seeding changelogs * Added unreleased section to CHANGELOG * Fix empty lines between sections * Add missing entries for abt * Update data to include latest releases * Update CHANGELOG.md * Add missing line in unreleased section for perf. * Enable CHANGELOG check globally (#4084) * Enable CHANGELOG check globally * Simplify conditional. * Enable COUNT integration tests, now that backend support has rolled out (#4163) * Remove separation of kotlin directories in dackka (#4166) * Deprecate App Check SafetyNet SDK (#4187) * Add `@Deprecated` annotations to Firebase App Check SafetyNet SDK public API. * Add `@deprecated` tag in the Javadoc as well. * Remove stale entries from Unreleased section. (#4185) * Assign ConfigContainer Builder return values. (#4194) * update bom (#4155) * update bom * update * update * feat(perf-ktx): add trace(name, block) extension function (#4180) * Remove smoke test for app indexing (#4219) App Indexing is deprecated starting BoM 31.0.0 . https://firebase.google.com/support/release-notes/android#bom_v31-0-0 * Bump Robolectric to 4.9 (#4161) * Add plexus-utils for firebase-database tests Looks like firebase-database tests use plexus-utils dependency of Robolectric directly. But this dependency was removed by Robolectric. So this CL adds plexus-utils explictly for firebase-database tests. Signed-off-by: utzcoz <utzcoz@outlook.com> * Bump Robolectric to 4.9 1. Use legacy LooperMode for tests explicitly, because recent Robolectric releases switch to use PAUSED mode default. Before these tests migrate to PAUSED mode, they use LEGACY mode to pass tests. 2. Migrate Assert.assertThat to Truth.assertThat to avoid using removed APIs. 3. All build.gradle use the same robolectricVersion except transport-backend-cct because Robolectric 4.8+ has compatibility problem for TelephonyManager with low compile/targetSdkversion. To keep httpclient compatibility, transport-backend-ccts continues to use Robolectric 4.3.1. 4. Remove unused exclude protobuf-java from Robolectric. 5. Add necessary protobuf-lite dependency on classpath for some ktx modules' tests. Signed-off-by: utzcoz <utzcoz@outlook.com> Signed-off-by: utzcoz <utzcoz@outlook.com> * return exception if modelname is empty (#4226) * Add "create release PR" github action (#4236) This implementation: - Creates the base branch (name is based in user input) - Creates the release branch (name is based in user input) - Creates the release.cfg file in the release branch without adding any SDK (module) to it. It can create the branches based on any existing branch of the repo. * Sync spec tests from web SDK to Android SDK (#4230) * Update versions (#4238) * Update versions * Exclude .github dir from `firebaseContinuousIntegration` paths (#4239) * Performing IN expansion (#4221) * WIP: `in` expansion. * Add composite filter in-expansion test. * Fix formatting. * Run in-expansion as part of DNF computation and add tests. * Add test with nested IN filters with CSI. * Add tests for other cases. * typo fix (#4237) * Firestore: Add test that verifies count query error message when missing index (#4232) * refactor(functions): update firebase-iid to 21.1.0 (#4225) * refactor(functions): update firebase-iid to 21.1.0 * Update CHANGELOG.md * Update CHANGELOG.md * bump firebase-iid-interop to 17.1.0 * exclude firebase-components from firebase-iid dependency Signed-off-by: utzcoz <utzcoz@outlook.com> Co-authored-by: Vladimir Kryachko <vkryachko@google.com> Co-authored-by: Daymon <17409137+daymxn@users.noreply.github.com> Co-authored-by: Raymond Lam <raymondlam@google.com> Co-authored-by: Yifan Yang <yifayan@gmail.com> Co-authored-by: Rosário Pereira Fernandes <rosariopf@google.com> Co-authored-by: emilypgoogle <110422458+emilypgoogle@users.noreply.github.com> Co-authored-by: Jeremy Jiang <64182239+jeremyjiang-dev@users.noreply.github.com> Co-authored-by: Rosalyn Tan <rosalyntan@google.com> Co-authored-by: Maneesh Tewani <maneesht@users.noreply.github.com> Co-authored-by: Matthew Robertson <mrober@users.noreply.github.com> Co-authored-by: wu-hui <53845758+wu-hui@users.noreply.github.com> Co-authored-by: Denver Coneybeare <dconeybe@google.com> Co-authored-by: Leo Zhan <zhanl@google.com> Co-authored-by: Rodrigo Lazo <rlazo@users.noreply.github.com> Co-authored-by: Dana Silver <danasilver@google.com> Co-authored-by: Vinay Guthal <vguthal@google.com> Co-authored-by: utzcoz <43091780+utzcoz@users.noreply.github.com> Co-authored-by: argzdev <alvinrustan@google.com> Co-authored-by: Mila <107142260+milaGGL@users.noreply.github.com> Co-authored-by: Ehsan <ehsannas@gmail.com> Co-authored-by: cherylEnkidu <96084918+cherylEnkidu@users.noreply.github.com> * Fix ImageUtils test after robolectric update * Remove extra member initialization in test app * Ignore flaky test Signed-off-by: utzcoz <utzcoz@outlook.com> Co-authored-by: Kai Bolay <kbolay@google.com> Co-authored-by: Vladimir Kryachko <vkryachko@google.com> Co-authored-by: Daymon <17409137+daymxn@users.noreply.github.com> Co-authored-by: emilypgoogle <110422458+emilypgoogle@users.noreply.github.com> Co-authored-by: Raymond Lam <raymondlam@google.com> Co-authored-by: Yifan Yang <yifayan@gmail.com> Co-authored-by: Rosário Pereira Fernandes <rosariopf@google.com> Co-authored-by: Jeremy Jiang <64182239+jeremyjiang-dev@users.noreply.github.com> Co-authored-by: Rosalyn Tan <rosalyntan@google.com> Co-authored-by: Maneesh Tewani <maneesht@users.noreply.github.com> Co-authored-by: Matthew Robertson <mrober@users.noreply.github.com> Co-authored-by: wu-hui <53845758+wu-hui@users.noreply.github.com> Co-authored-by: Denver Coneybeare <dconeybe@google.com> Co-authored-by: Leo Zhan <zhanl@google.com> Co-authored-by: Rodrigo Lazo <rlazo@users.noreply.github.com> Co-authored-by: Dana Silver <danasilver@google.com> Co-authored-by: Vinay Guthal <vguthal@google.com> Co-authored-by: utzcoz <43091780+utzcoz@users.noreply.github.com> Co-authored-by: argzdev <alvinrustan@google.com> Co-authored-by: Mila <107142260+milaGGL@users.noreply.github.com> Co-authored-by: Ehsan <ehsannas@gmail.com> Co-authored-by: cherylEnkidu <96084918+cherylEnkidu@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make a best effort attempt to flush reports at crash time. This should allow us to upload reports for app initialization crashes. This will also prevent the app from needing to relaunch to upload the scheduled reports. If the best effort attempt fails, this will still go back to the original behaviour.