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

Error when throwing exception in getChildren #1081

Open
dsst95 opened this issue Jul 14, 2024 · 5 comments
Open

Error when throwing exception in getChildren #1081

dsst95 opened this issue Jul 14, 2024 · 5 comments

Comments

@dsst95
Copy link

dsst95 commented Jul 14, 2024

Feature proposal

It should be possible to show an custom error in full screen in Android Auto. When an error occurs during AudioHandler.getChildren, result.sendError is used which leads to another exception:

java.lang.UnsupportedOperationException: It is not supported to send an error for root
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at androidx.media.MediaBrowserServiceCompat$Result.onErrorSent(MediaBrowserServiceCompat.java:945)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at androidx.media.MediaBrowserServiceCompat$Result.sendError(MediaBrowserServiceCompat.java:890)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at com.ryanheise.audioservice.AudioServicePlugin$AudioHandlerInterface$1.error(AudioServicePlugin.java:539)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at io.flutter.plugin.common.MethodChannel$IncomingResultHandler.reply(MethodChannel.java:246)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at io.flutter.embedding.engine.dart.DartMessenger.handlePlatformMessageResponse(DartMessenger.java:376)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at io.flutter.embedding.engine.FlutterJNI.handlePlatformMessageResponse(FlutterJNI.java:1076)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at android.os.MessageQueue.nativePollOnce(Native Method)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at android.os.MessageQueue.next(MessageQueue.java:343)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at android.os.Looper.loop(Looper.java:188)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at android.app.ActivityThread.main(ActivityThread.java:7617)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at java.lang.reflect.Method.invoke(Native Method)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
E/MethodChannel#com.ryanheise.audio_service.handler.methods(24285): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:941)

Setting the PlayerState to error and returning null from AudioHandler.getChildren is also not possible. But in order to show an full screen error message null must be returned from getChildren.

Motivating use case(s)

I am trying to show an custom full screen error to the user if he is not logged in the app in Android Auto, as described in https://developer.android.com/training/cars/media/automotive-os#-error-handling. But there is no possibility to show an custom error from AudioHandler.getChildren.

@harpreetslabs
Copy link

This is due to a null check operator error in the getChildren function.
return _mediaLibrary.items[parentMediaId]!;
You can use try catch in getChildren to find exact error or can use this way:
return (_mediaLibrary.items[parentMediaId] ?? []);

@ryanheise
Copy link
Owner

Setting the PlayerState to error and returning null from AudioHandler.getChildren is also not possible. But in order to show an full screen error message null must be returned from getChildren.

I can add this, although since it would be a breaking change, it will go in the major branch and I'll need to consider what else to include along with it.

@dsst95
Copy link
Author

dsst95 commented Dec 9, 2024

Setting the PlayerState to error and returning null from AudioHandler.getChildren is also not possible. But in order to show an full screen error message null must be returned from getChildren.

I can add this, although since it would be a breaking change, it will go in the major branch and I'll need to consider what else to include along with it.

That would be great! Should I adjust my PR, or will you implement it on your own?

@ryanheise
Copy link
Owner

There may be some value in separating the changes into non-breaking (to be merged into minor) and breaking (to be merged into major) and get a new minor release out first.

@ryanheise
Copy link
Owner

Just thinking on this, the non-breaking version of this would be to make it sot hat if getChildren throws an exception, the Dart side can catch that and send a null to the platform side.

In the major version, we can then change the API to allow returning null.

Following the docs, the idea would be that before the getChildren implementation either throws the exception or returns null, it should broadcast a new playbackState with the errorCode and errorMessage set appropriately. Someone would probably need to test this to ensure that the playbackState is actually broadcast on the platform side before onLoadChildren returns.

@dsst95 Since you have a PR in progress, I'd be happy to let you implement this, but I'd be just happy to implement it. Note that there would need to be some null-checking on the platform side for onLoadChildren, but I think onLoadItem already looks fine if we take the same approach and catch exceptions on the Dart side and translating them into null results so that errors never get transmitted over the method channel for these particular APIs (given that the Android APIs are not supposed to throw exceptions, only return nulls instead.)

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 a pull request may close this issue.

3 participants