Skip to content

fix(android): catch ForegroundServiceStartNotAllowedException in back…#311

Open
dishit-wednesday wants to merge 4 commits into
mainfrom
fix-foreground-crahes
Open

fix(android): catch ForegroundServiceStartNotAllowedException in back…#311
dishit-wednesday wants to merge 4 commits into
mainfrom
fix-foreground-crahes

Conversation

@dishit-wednesday
Copy link
Copy Markdown
Collaborator

…ground

Summary

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (code change that neither fixes a bug nor adds a feature)
  • Chore (build process, CI, dependency updates, etc.)

Screenshots / Screen Recordings

Android

Before After

iOS

Before After

Checklist

General

  • My code follows the project's coding style and conventions
  • I have performed a self-review of my code
  • I have added/updated comments where the logic isn't self-evident
  • My changes generate no new warnings or errors

Testing

  • I have tested on Android (physical device or emulator)
  • I have tested on iOS (physical device or simulator)
  • I have tested in light mode and dark mode
  • Existing tests pass locally (npm test)
  • I have added tests that prove my fix is effective or my feature works

React Native Specific

  • No new native module without corresponding platform implementation (Android + iOS)
  • New native modules are added to the Xcode project build target (project.pbxproj)
  • No hardcoded pixel values — uses SPACING / TYPOGRAPHY constants from the theme
  • Styles use useThemedStyles pattern (not inline or static StyleSheet.create)
  • Animations/gestures work smoothly on both platforms
  • Large lists use FlatList / FlashList (not .map() inside ScrollView)
  • No unnecessary re-renders introduced (check with React DevTools Profiler if unsure)

Performance & Models

  • Downloads / long-running tasks report progress to the UI
  • File paths are resolved correctly on both platforms (no hardcoded / vs \\)
  • Large files (models, assets) are not committed to the repository

Security

  • No secrets, API keys, or credentials are included in the code
  • User input is validated/sanitized where applicable

Related Issues

Additional Notes

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
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 wraps the service start logic in a try-catch block to handle background start restrictions on Android 12+. The review feedback suggests catching IllegalStateException instead of a generic Exception to ensure that unrelated runtime errors, such as missing permissions, are not suppressed.

} else {
context.startService(intent)
}
} catch (e: Exception) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Catching a generic Exception is overly broad and can swallow unrelated runtime errors (such as SecurityException if the FOREGROUND_SERVICE permission is missing) that should be surfaced during development. Since the primary goal is to handle background start restrictions, catching IllegalStateException is more appropriate. On Android 12+ (API 31), ForegroundServiceStartNotAllowedException inherits from IllegalStateException, so this change will correctly handle the intended case while allowing other critical exceptions to be identified.

Suggested change
} catch (e: Exception) {
} catch (e: IllegalStateException) {

@sonarqubecloud
Copy link
Copy Markdown

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.

1 participant