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

Android 12 Catch ForegroundServiceStartNotAllowedException #1823

Merged

Conversation

JulianKast
Copy link
Contributor

@JulianKast JulianKast commented Jul 26, 2022

Fixes #1815

This PR is [ready] for review.

Risk

This PR makes [minor] API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).
  • I have tested Android

Core Tests

Tested against Sync 3.0 with Android 12 device and Android 9 device.
Android 12 device:
I was able to replicate the issue by using a delayed handler. I had to put the try-catch inside of the handler to catch it.

                final Handler handler = new Handler(Looper.getMainLooper());
                handler.postDelayed(new Runnable() {
                    @Override
                    public void run() {
                        try {
                            serviceIntent.putExtra(FOREGROUND_EXTRA, true);
                            DebugTool.logInfo(TAG, "Attempting to startForegroundService - " + System.currentTimeMillis());
                            setForegroundExceptionHandler(); //Prevent ANR in case the OS takes too long to start the service
                            context.startForegroundService(serviceIntent);
                        } catch (ForegroundServiceStartNotAllowedException e) {
                            Log.e(TAG, "ForegroundServiceStartNotAllowedException herer");
                        }

                    }
                }, 25000);

To catch it where it is now we can do that by throwing the exception where we call context.startForegroundService(serviceIntent); You will also need to add @RequiresApi(api = Build.VERSION_CODES.S) when testing it this way.

throw new ForegroundServiceStartNotAllowedException("msg");

Android 9 device:
Connected Hello sdl without the modifications above to ensure that the code ran as intended.

Summary

Android 12 may throw a ForegroundServiceStartNotAllowedException if we try to start the router service after a certain amount of time from receiving a broadcast that requires the BLUETOOTH_CONNECT or BLUETOOTH_SCAN permission. In testing, I saw it happen between 20 to 25 seconds.

Changelog

Bug Fixes
  • Catch ForegroundServiceStartNotAllowedException in two places in SdlBroadcastReceiver.

CLA

@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #1823 (4f8e483) into develop (ccf0af9) will increase coverage by 0.25%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1823      +/-   ##
=============================================
+ Coverage      53.78%   54.04%   +0.25%     
- Complexity      5519     5532      +13     
=============================================
  Files            562      562              
  Lines          25809    25821      +12     
  Branches        3395     3398       +3     
=============================================
+ Hits           13881    13954      +73     
+ Misses         10660    10596      -64     
- Partials        1268     1271       +3     
Impacted Files Coverage Δ
...martdevicelink/transport/SdlBroadcastReceiver.java 3.02% <0.00%> (-0.12%) ⬇️
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 52.43% <0.00%> (+1.21%) ⬆️
...rc/main/java/com/android/grafika/gles/EglCore.java 44.82% <0.00%> (+2.58%) ⬆️
.../java/com/android/grafika/gles/EglSurfaceBase.java 32.14% <0.00%> (+7.14%) ⬆️
...smartdevicelink/encoder/VirtualDisplayEncoder.java 45.69% <0.00%> (+9.60%) ⬆️
...main/java/com/android/grafika/gles/Drawable2d.java 67.92% <0.00%> (+11.32%) ⬆️
...ava/com/android/grafika/gles/Texture2dProgram.java 59.18% <0.00%> (+24.48%) ⬆️
...n/java/com/android/grafika/gles/FullFrameRect.java 55.00% <0.00%> (+30.00%) ⬆️

Copy link
Member

@joeygrover joeygrover left a comment

Choose a reason for hiding this comment

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

I was still getting errors in the IDE even with the suppress lint calls. I think we can get around those still. Try adding a new method like the one I have included to handle the exceptions and use version checks within in it. Then in the original catch statement add catching for IllegalStateException since that is a parent exception that is included in lower versions of Android. In case there are other extensions of IllegalStateException that can be thrown we will simply catch them and print out that the reason was unknown. This should probably help us into the future prevent crashes when Android decides to add more state requirements.

The exception handler

    private static void handleStartServiceException(Exception e) {
        if (e instanceof SecurityException) {
            DebugTool.logError(TAG, "Security exception, process is bad");
            return;
        } else if (Build.VERSION.SDK_INT > Build.VERSION_CODES.R) {
            if (e instanceof ForegroundServiceStartNotAllowedException) {
                DebugTool.logError(TAG, "Not allowed to start service in foreground");
                return;
            } else if (e instanceof ServiceStartNotAllowedException) {
                DebugTool.logError(TAG, "Not allowed to start service in current state");
                return;
            }
        }
        DebugTool.logError(TAG, "Unable to start service for unknown reason");
    }

Refactored exception catching

        } catch (SecurityException | IllegalStateException e) {
            handleStartServiceException(e);
            // This service could not be started
        }

@joeygrover joeygrover merged commit 3fd2499 into develop Aug 15, 2022
@joeygrover joeygrover deleted the bugfix/issue_1815_ForegroundServiceStartNotAllowedException branch August 15, 2022 18:40
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