fix: Crashlytics defer API calls until async context initialization completes#15879
fix: Crashlytics defer API calls until async context initialization completes#15879
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a race condition where Crashlytics API calls could be silently dropped if made immediately after initialization. The approach of deferring calls using a promise until the asynchronous initialization is complete is solid, and the changes correctly wrap several public API methods. While the fix is good, it appears that some other public methods that could also be affected by this race condition might have been missed. Methods like recordError:userInfo:, recordExceptionModel:, and recordOnDemandExceptionModel: also seem to perform actions that depend on the context being initialized, but they are not wrapped. To ensure a comprehensive fix, I recommend reviewing these methods and wrapping them with waitForContextInit: as well. On a minor note, the added warning log and the typo fix are good improvements.
Updated code to defer execution of |
ncooke3
left a comment
There was a problem hiding this comment.
Nice fix!
One possible nit, if multiple key/value pairs are expected, then the logs should read Failed to [read|write] key/value pairs
Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
|
Failing tests are known flakes. |
Fixes a silent failure where Crashlytics API calls are dropped when invoked immediately after Firebase initialization.
Explanation
Recently, Crashlytics transitioned to asynchronous initialization to improve overall app startup performance. Because of this change, when
FirebaseApp.configure()finishes executing on the main thread, the Crashlytics SDK may still be actively initializing its core context on a background thread.If a developer calls a Crashlytics API method (such as
setUserID:) right after configuring Firebase, the method executes while the Crashlytics context is not yet fully ready. Previously, this resulted in the SDK simply dropping the request. Because there were no logs emitted when this occurred, it created a silent failure.Approach
To resolve this race condition, the affected public API methods were updated to wrap their internal logic inside the existing
waitForContextInit:callback:private helper.Instead of executing synchronously and failing, the API calls now check the
_contextInitPromise. If the background initialization is still in progress, the API execution block is chained to the promise and safely deferred until the setup completes.