Skip to content

Intercom not initialized crash#4542

Closed
aaravgarg wants to merge 2 commits intomainfrom
fix/android-crash-intercom-not-initialized
Closed

Intercom not initialized crash#4542
aaravgarg wants to merge 2 commits intomainfrom
fix/android-crash-intercom-not-initialized

Conversation

@aaravgarg
Copy link
Collaborator

Summary

  • Add _initialized flag to IntercomManager to guard all Intercom calls
  • Prevents "Intercom not initialized" crash when Intercom methods are called before initIntercom() completes
  • The _isIntercomEnabled check now includes the initialization flag, so all operations silently skip if not yet initialized

Crash Stats

Test plan

  • Verify Intercom still works after app initialization
  • Test rapid app startup where Intercom calls may happen before init completes

🤖 Generated with Claude Code

Add _initialized flag to prevent "Intercom not initialized" crash when
Intercom methods are called before initIntercom() completes. All
Intercom operations now check _initialized before proceeding.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@aaravgarg aaravgarg requested a review from mdmohsin7 February 1, 2026 23:24
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses the 'Intercom not initialized' crash by introducing an _initialized flag to guard against race conditions. The approach is sound, however, I've found a critical logic bug in the initIntercom method that could leave the manager in an incorrect state. Additionally, there's an opportunity to refactor some duplicated logic to improve maintainability. My review includes specific comments and suggestions to address these points.

Comment on lines 28 to 39
Future<void> initIntercom() async {
if (Env.intercomAppId == null) return;
return PlatformService.executeIfSupportedAsync(
_isIntercomEnabled,
await PlatformService.executeIfSupportedAsync(
PlatformService.isIntercomSupported && (Env.intercomAppId != null && Env.intercomAppId!.isNotEmpty),
() => intercom.initialize(
Env.intercomAppId!,
iosApiKey: Env.intercomIOSApiKey,
androidApiKey: Env.intercomAndroidApiKey,
),
);
_initialized = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The early return on line 29 if Env.intercomAppId is null prevents _initialized from ever being set to true. This leaves the IntercomManager in a perpetual 'initializing' state, which is incorrect as the initialization process has completed (by doing nothing).

The check for Env.intercomAppId != null on line 31 is sufficient. By removing the early return, _initialized will be correctly set to true after the initialization attempt, and _isIntercomEnabled will correctly evaluate to false if Intercom is not configured, preventing any calls.

  Future<void> initIntercom() async {
    await PlatformService.executeIfSupportedAsync(
      PlatformService.isIntercomSupported && (Env.intercomAppId != null && Env.intercomAppId!.isNotEmpty),
      () => intercom.initialize(
        Env.intercomAppId!,
        iosApiKey: Env.intercomIOSApiKey,
        androidApiKey: Env.intercomAndroidApiKey,
      ),
    );
    _initialized = true;
  }

Comment on lines 19 to +22
bool get _isIntercomEnabled =>
PlatformService.isIntercomSupported && (Env.intercomAppId != null && Env.intercomAppId!.isNotEmpty);
_initialized &&
PlatformService.isIntercomSupported &&
(Env.intercomAppId != null && Env.intercomAppId!.isNotEmpty);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

To improve readability and avoid duplicating logic, consider extracting the condition for whether Intercom is configured into its own private getter. This condition is used here and also in initIntercom.

  bool get _isIntercomConfigured =>
      PlatformService.isIntercomSupported &&
      (Env.intercomAppId != null && Env.intercomAppId!.isNotEmpty);

  bool get _isIntercomEnabled => _initialized && _isIntercomConfigured;

You would then also use _isIntercomConfigured inside initIntercom.

Set _initialized=true on unsupported platforms and when API key is empty
so methods degrade gracefully. Only leave _initialized=false on actual
initialization failure.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mdmohsin7
Copy link
Member

fixed in #4624

@mdmohsin7 mdmohsin7 closed this Feb 5, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

Hey @aaravgarg 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

  • Project standards — Ensuring consistency across the codebase
  • User needs — Making sure changes align with what our users need
  • Code best practices — Maintaining code quality and maintainability
  • Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants