Skip to content

ScheduleManager.saveSchedule - EXC_BAD_ACCESS crash#4534

Closed
aaravgarg wants to merge 1 commit intomainfrom
fix/ios-crash-schedule-manager-exc-bad-access
Closed

ScheduleManager.saveSchedule - EXC_BAD_ACCESS crash#4534
aaravgarg wants to merge 1 commit intomainfrom
fix/ios-crash-schedule-manager-exc-bad-access

Conversation

@aaravgarg
Copy link
Collaborator

Summary

  • Wrap AwesomeNotifications.createNotification call in try-catch
  • Prevents native EXC_BAD_ACCESS crash in ScheduleManager.saveSchedule (internal AwesomeNotifications pod method)
  • The crash occurs in the native iOS notification scheduling code, this fix catches the exception at the Dart bridge level

Crash Stats

Test plan

  • Verify notifications still show correctly
  • Verify notification scheduling still works

🤖 Generated with Claude Code

Prevent native EXC_BAD_ACCESS crash in ScheduleManager.saveSchedule
by catching exceptions from the AwesomeNotifications plugin.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@aaravgarg aaravgarg requested a review from mdmohsin7 February 1, 2026 23:17
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 introduces a try-catch block around the AwesomeNotifications.createNotification call to prevent a native crash on iOS. This is a good defensive measure. My review includes a suggestion to improve the error handling within the new catch block to ensure that these failures are properly logged and tracked, rather than being silently ignored in debug logs.

Comment on lines +108 to +110
} catch (e) {
Logger.debug('Failed to create notification: $e');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

high

While adding a try-catch block is the correct approach to prevent the app from crashing, logging the error at the debug level is insufficient for an issue that was causing a native crash. Such failures should be logged as errors to ensure they are tracked in error monitoring services like Crashlytics. This will help in monitoring the frequency of this failure and verifying that the fix is effective.

Please change the catch block to capture the stack trace and use Logger.handle to report it as a proper error.

Suggested change
} catch (e) {
Logger.debug('Failed to create notification: $e');
}
} catch (e, s) {
Logger.handle(e, s, message: 'Failed to create notification');
}

@mdmohsin7
Copy link
Member

closed in #4628

@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