Skip to content
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

b/151167694 - sendUnsentReport might not upload until the second time the API is called #5060

Merged
merged 2 commits into from
Mar 11, 2020

Conversation

jasonhu-g
Copy link
Contributor

Scenario:

  1. Developer has automatic data collection disabled inside the app plist
  2. A crash occurred
  3. The app relaunches and the developer calls sendUnsentReport

Result:

  1. The crash report does not get uploaded because there's a race condition:
  • Settings is triggered async
  • The report is not uploaded because the org id from settings is not available

Validation:

  1. E2E scenario
  2. Unit tests
  3. Verified even when FIRCrashlytics.sendUnsentReport is on the main thread, waiting for the semaphore is not on the main thread.

@jasonhu-g jasonhu-g merged commit f64ff92 into master Mar 11, 2020
@jasonhu-g jasonhu-g deleted the crashlytics-fixSendUnsentReports branch March 11, 2020 18:41
doudounan pushed a commit that referenced this pull request Mar 19, 2020
* Move UpdateFirebasePod to Swift's ArgumentParser library. (#4993)

* Move UpdateFirebasePod to Swift's ArgumentParser library.

This was introduced in URL, leaving a nice clean way to provide command
line arguments.

Example output:

```
$ swift run UpdateFirebasePod
Error: Missing expected argument '--git-root <git-root>'
Usage: firebase-pod-updater --git-root <git-root> --current-release <current-release>

```

* Removed unnecessary file.

* Fixed formatting.

* Use existing argument name for currentRelease.

* Sorted package dependencies.

* Pin to exact version.

* Rename UpdateFirebasePod to firebase-pod-updater

This sticks with convention expected by the tooling. Future PRs will change the `ReleasePackager` and other tools as well.

* Renamed pod updater. (#5003)

* Upload Symbols 3.1 (#4988)

* Crashlytics Update CHANGELOG.md for v4.0.0-beta.5 (#5004)

* Checks for connectivity when App is moving to foreground. (#4985)

* Checks for connectivity when App is moving to foreground.

* Split MaybeInvokeCallbacks and invoke them when foregrounding and network is available.

* Update CHANGELOG for Firestore v1.11.1 (#5008)

* Support AB testing of Firebase In-App Messages (#5005)

* Parse out experiment payload for experimental FIAMs and store it on FIRIAMMessageDefinition

* Fix bugs in parsing experimental payload

* Add nullable experiment payload to FIAM message definition. Add method in FIRExperimenmtController to validate running experiments and call this when new messages are fetched from FIAM backend

* Add and document new ABT methods, update FirebaseInAppMessaging.podspec to depend on ABT

* Clean up deprecated public message initializers, add experiment payload to private initializers

* Activate experiment for experimental messages upon impression

* Clean up activateExperiment API

* Add experimental payload to all message definition initializers

* Add unit test coverage for new ABT methods

* Add test coverage for parsing experimental messages

* Verify experiment payload gets passed to message superclass

* Scripts/style.sh

* Update Podfile for UI test app to include AB Testing SDK

* Add back deprecated initializers for now, these are needed for the test app

* Test for non-nil experiment payload

* Use mutable array directly, better memory usage

* Bump ABTesting podspec version, depend on new ABT in FIAM

* Remove mentions of FIREventOrigins.h

* Remove experiment payload from public API, hide it in private

* Scripts/style.sh

* Fix comment indentation, allow for FIAM to depend on minor version updates of ABTesting

* Move ABTExperimentPayload import to private header

* Core M66 changelog update (#5013)

* Core M66 changelog update

* Core M65 notes corrected.

* FirebaseStorage.podspec - update Core dependency version to one supporting watchOS (#5014)

* Update versions for Release 6.19.0

* Add FirebaseABTesting to travis cron tests (#5015)

* GoogleDataTransportCCTSupport - bump GoogleDataTransport dependency to ~> 5.0

* FirebaseCoreDiagnostics 1.2.2 - bump GoogleDataTransportCCTSupport dependency to ~> 2.0 (#5016)

* M66 FirebaseCoreDiagnostics added to the release manifest (#5018)

* M66 FIAM GoogleDataTransportCCTSupport dependency bump to '~> 2.0' (#5019)

* update storage changelog (#5017)

* Integrating GoogleDataTransport in Crashlytics to Report Crashes  (#4989)

* Remove setting CMAKE_BUILD_TYPE in CMakeLists.txt (#5027)

This really needs to be set on the command-line if it's to be effective,
and setting it here breaks downstream projects that include
firebase-ios-sdk as a subdirectory.

* Consolidate the Google Analytics origin for Crashlytics. Only use "clx". (#5030)

* Fix assertions not printing correctlty (#5026)

* M66 changelog for FIAM and ABT SDKs (#4920)

* M65 changelog for ABT SDK

* Improve CHANGELOG notes

* Fix CHANGELOG version

* M66 changelog for both FIAM & ABT

* Fix formatting

* Fix formatting pt2

* Put deprecated method in backticks

* add Installations to docs script (#5035)

* Update GDT and GDTCCT CHANGELOGs (#5034)

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG for M66. (#5036)

* Update CHANGELOG for M66.

* Update CHANGELOG.md

* Fix typos in doc comments (#5038)

* Cherry Pick Crashlytics Upload Symbols (#5046)

* Fix compilation under Linux/GCC 9.2.1 in Release mode (#5042)

The `firebase_firestore_protos_libprotobuf` library contains
protoc-generated code. Compile with standard flags rather than
Firestore's default of `-Wall -Wextra -Werror`.

* Fix nightly zip GHA build (#5041)

* CocoaPods 1.9.1 (#5050)

* Cherry Pick Crashlytics Upload Symbols (#5046) (#5047)

Co-authored-by: Sam Edson <[email protected]>

* MFA beta (#4823)

* Fix a usage of `Mutation` at a point where it's only been forward-declared (#5037)

* Address a few potential security issues (#5059)

* Address a few potential security issues

Eliminates use of memcpy and malloc

* Update CHANGELOG

* Merge release 6.19.0 (#5058)

* Update versions for Release 6.19.0

* GoogleDataTransportCCTSupport - bump GoogleDataTransport dependency to ~> 5.0

* FirebaseCoreDiagnostics 1.2.2 - bump GoogleDataTransportCCTSupport dependency to ~> 2.0 (#5016)

* M66 FirebaseCoreDiagnostics added to the release manifest (#5018)

* M66 FIAM GoogleDataTransportCCTSupport dependency bump to '~> 2.0' (#5019)

* Cherry Pick Crashlytics Upload Symbols (#5046) (#5047)

Co-authored-by: Sam Edson <[email protected]>

* M66: Crashlytics dependency versions update.

Co-authored-by: Sam Edson <[email protected]>

* Update environment checks (#5056)

* update environment checks

* replace deprecated macro

* update GoogleUtilities changelog

* Refactor and clean up the CMake build ahead of CMake/iOS support (#5051)

* Use early exit in objc_framework CMakeLists.txt

* Clean up objc_framework CMakeLists.txt

  * Use `version` consistently
  * Use `firebase_ios_version` for the repo-level version
  * Remove INCLUDE directories that are no longer needed
  * Sort dependencies

* Simplify Example/App/CMakeLists.txt

  * Add `OBJC_FLAGS` directly, avoiding a separate `sources` variable.
  * Add resources directly

* Rename test host app to firebase_firestore_example_app

* Add a firebase_ios prefix to functions and variables

This reduces the chance of collision with other projects.

* Simplify compiler_setup

Now that compiler_setup.cmake no longer has side-effects, fold
compiler_id.cmake and archive_options.cmake into it and move
the include of compiler_setup up to the top of the main build.

* Capitalize (#5063)

* Explicitly state Firestore's dependency on UIKit (#5067)

In #4985 we added support for listening to
`UIApplicationWillEnterForegroundNotification` notifications to better
handle situations where we may have missed callbacks from
`SCNetworkReachabilitySetCallback` while Firestore was in the background.

Fixes #5065.

* Explicitly state Firestore's dependency on UIKit (#5077)

* Explicitly state Firestore's dependency on UIKit (#5067)

In #4985 we added support for listening to
`UIApplicationWillEnterForegroundNotification` notifications to better
handle situations where we may have missed callbacks from
`SCNetworkReachabilitySetCallback` while Firestore was in the background.

Fixes #5065.

* Update CHANGELOG for Firestore v1.11.2

* b/151167694 - sendUnsentReport might not upload until the second time the API is called (#5060)

* Fix fuzz build (#5069)

It was broken by our transition to avoid Abseil types in signatures.

* Remove cleanup of experiments that are no longer running (#5078)

* Remove cleanup of experiments that are no longer running (product decision)

* Revert podspec bump

* Bump Auth pod version (#5064)

* Don't attempt to create data if the event's fileURL is nil. (#5088)

* Don't attempt to create data if the event's fileURL is nil.

This could be because of changes to NSCoding w.r.t. transitioning from GDTCORStoredEvent to GDTCOREvent.

* Attempt to address sudden analyzer issues

* Don't pass nil to a function that requires non-nil arguments.

* Change error code to the new GDTCORMCEFileReadError

* Update CHANGELOG.md (#5094)

* Remove smart quotes (#5090)

* Update M66 CHANGELOG (#5039)

* Update M66 CHANGELOG

* Update CHANGELOG

* Add iOS build support to the CMake build (#5052)

* Use early exit in objc_framework CMakeLists.txt

* Clean up objc_framework CMakeLists.txt

  * Use `version` consistently
  * Use `firebase_ios_version` for the repo-level version
  * Remove INCLUDE directories that are no longer needed
  * Sort dependencies

* Simplify Example/App/CMakeLists.txt

  * Add `OBJC_FLAGS` directly, avoiding a separate `sources` variable.
  * Add resources directly

* Rename test host app to firebase_firestore_example_app

* Add a firebase_ios prefix to functions and variables

This reduces the chance of collision with other projects.

* Simplify compiler_setup

Now that compiler_setup.cmake no longer has side-effects, fold
compiler_id.cmake and archive_options.cmake into it and move
the include of compiler_setup up to the top of the main build.

* Add support for building for iOS

* Make building benchmarks optional

* Disable protoc-based source generators when cross-compiling

* Make downloading googletest optional

* Add DISABLE_STRICT_WARNINGS mode to cc_rules

This makes it possible to build non-Firestore components in the CMake
build to a lower standard than that to which we hold ourselves without
sacrificing the utility of the various build rules that we have.

* Move to xcframework bundles (#4737)

* Enable Mac Catalyst in support project

* Update builder to output xcframework

Replaces fat framwork and adds Mac Catalyst support
Note: This removes i386 and armv7 support, as xcframework doesn’t support these

* Add Modile and Info.plist fles

* Generate real dynamic frameworks

* Remove unnecessary file

* Preserve symlinks for Mac SDK

* Fix typo

* Re-enable armv7 and i386

* Green CI for xcframework-master branch (#4778)

* Static library framework build for xcframeworks zip (#4799)

* Build xcframeworks for pods with resources (#4911)

* Make Zip Builder temp dirs sortable and understandable with timestamp (#4919)

* Zip Builder support for dependencies through subspecs (#4940)

* Restore Firestore build (#4995)

* Use static frameworks to build zip (#5029)

* Add Swift module support to ZipBuilder (#5040)

* Zip Carthage refactor (#5080)

* Zip Resource handling for xcframeworks (#5093)

* Add Crashlytics to if_changed for GoogleDataTransport changes (#5102)

* FirebaseAuth should be in implicit pod list (#5104)

* Use FIRInstallations SDK to fetch FID and FIS token, send those values in requests to FIAM backend (#5081)

* Depend on FIRInstallations instead of IID, load an installations object into FIRInAppMessagingPrivate.h

* Bootstrap FIRInAppMessaging.h with an instance of FIRInstallations

* Update FIRIAMClientInfoFetcher to use FIRInstallations method to grab FID and FIS Token

* Call new client info fetcher method

* Fix up tests

* Change nullability for FIRInstallations object, handle null case

* More verbose comments in FIRIAMClientInfoFetcher, call completion in case that FIS token is fetched, but FID fails

* Throw error and call completion if Firebase Installations object is not created

* Send FIS token with method fetch requests

* Add test class for FIRIAMClientInfoFetcher

* Refactor FIRIAMClientInfoFetcher to get a FIRInstallations object injected

* Add unit testing for fetchFirebaseInstallationDataWithProjectNumber: in FIRIAMClientInfoFetcher

* Streamline unit tests, add case for nil FirebaseInstallations, clean up comments that were left in

* Fix Firebase/Auth version (#5106)

* Remote Config: enable ARC for tests. (#5108)

* Remote Config: enable ARC for tests.

* run ./scripts/style.sh

* Fix warnings

* Attempt to fix intermittent failing Crashlytics settings tests in CI (#5107)

* GoogleUtilities 6.5.2 (#5110)

* Crashlytics add new Record Exception Model API (#5055)

* Installations: access FIRHeartbeatInfo from a background thread (#5114)

* FIS: assert heartbeat storage is not accessed in main thread.

* FIS: Access hearbeat storage from a background queue.

* Make the prioritizer save state between app launches (#5120)

* Revert "Address a few potential security issues (#5059)" (#5121)

This reverts commit 86189f7.

* The uploader should retry when there's an error (#5116)

* The uploader should retry when there's an error

* Log the decoding error if there's one

* Migrate from using IID SDK for Remote Config to the new FIS SDK (#5096)

* Update podspec to replace IID with installations SDK.

* Update sample app podfile to include installations sdk.

* Source code changes to support migration to FIS. The FIS SDK now supports multi-app FIS ids and tokens.

* Update RC test app to use FIS SDK.

* WIP: Unit tests for incorporating FIS with Remote Config.

* Remote Config: fix Installations tests (#5109)

* Fix Installations tests.

* Fix Firebase/Auth version (#5106)

* Remote Config: enable ARC for tests.

* run ./scripts/style.sh

* Fix warnings

* Remote Config: enable ARC for tests. (#5108)

* Remote Config: enable ARC for tests.

* run ./scripts/style.sh

* Fix warnings

Co-authored-by: Paul Beusterien <[email protected]>

* Add the installations token as an HTTP header.

* Add changelog for FIS changes.

Co-authored-by: Maksym Malyhin <[email protected]>
Co-authored-by: Paul Beusterien <[email protected]>

* Merge release 6.20.0 (#5118)

* FIS changelog M67 (#5125)

* Add Catalyst testing for GHA (#5115)

* Crashlytics changelog v4.0.0-beta.6 (#5117)

* Crashlytics upload-symbols 3.2 (#5129)

* Enable lint for Auth (#5097)

* In banner view, pin top elements to the bottom of the status bar (#5100)

* Add constraint to pin title label below status bar in banner FIAM

* Remove title label top pin from storyboard at build time

* Use conditional compilation for iOS 13 check

* Release notes for M67 (#5127)

* Crashlytics cache key app version (#5132)

* Pin Crashlytics to GoogleDataTransport 5.0.1 and GoogleDataTransportC… (#5133)

* Fix deprecation warnings by consolidating coding calls (#5131)

* Fix deprecation warnings by consolidating coding calls

Fixes #5086

* Fix all NSSecureCoding references

* Fix changed API usage

* Update GDT versions, deps, and CHANGELOGs (#5138)

* M67 Core Changelog (#5135)

* Default to use new report upload endpoint when settings were failed to be fetched (#5128)

* Fix missing path specs for pods in repo (#5140)

* Refactor event fileURLs to be derived (#5137)

* Refactor event fileURLs to be derived

Apparently, since iOS 8, it's possible for the app's bundle to change directories as often as every launch. This is a mechanism that's hopefully slightly more robust against that, as data is supposed to carry over, the UUID component of the app's absolute directory might just change.

* Convert a couple of tests, actually use a non-absolute URL in GDTCOREvent

* Make sure to use the event fileURL property as the source of truth.

Add some additional logs and improve the integration test

* Fix KVC not working for the removed property

* Dynamic Links: cleanup Apple framework dependencies (#5141)

* Dynamic Links changelog M64 M67 (#5142)

* Fix and enable flakey Crashlytics tests (#5145)

* Fix changelog typo (#5146)

* Libraries should save state if a completion block is requested (#5147)

* Libraries should save state if a completion block is requested

Currently, the onComplete block of sendXEvent:onComplete: is invoked when the event bytes are written to disk. However, if the app crashes before a lifecycle event occurs (trigger GDTCORStorage to serialize to disk), that event if effectively lost because the metadata has all been lost. This change triggers the saving of state of GDTCORStorage and prioritizers (if they implement it) if an onComplete block has been given.

* Remove flaky check

* Update GDTCORStorage.m

* Make `api::CollectionReference::collection_id` return a const reference

Primarily for consistency with `DocumentReference::document_id`. Note that `BasePath::last_segment` returns a const reference.

Co-authored-by: Ryan Wilson <[email protected]>
Co-authored-by: Sam Edson <[email protected]>
Co-authored-by: wu-hui <[email protected]>
Co-authored-by: Gil <[email protected]>
Co-authored-by: christibbs <[email protected]>
Co-authored-by: Maksym Malyhin <[email protected]>
Co-authored-by: Paul Beusterien <[email protected]>
Co-authored-by: Chen Liang <[email protected]>
Co-authored-by: Jason Hu <[email protected]>
Co-authored-by: Michael Haney <[email protected]>
Co-authored-by: Morgan Chen <[email protected]>
Co-authored-by: dmandar <[email protected]>
Co-authored-by: Sam Edson <[email protected]>
Co-authored-by: Chuan Ren <[email protected]>
Co-authored-by: Konstantin Varlamov <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
@firebase firebase locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants