Enhancement/1366-prevent-crash-form-happening

[TimHoogstrate]

Resolves 1366 by adding an onFailed listener and catching the error.

Pre-launch Checklist

• I made sure the project builds.
• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is does not need version changes.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I rebased onto main.
• I added new tests to check the change I am making, or this PR does not need tests.
• I made sure all existing and new tests are passing.
• I ran dart format . and committed any changes.
• I ran flutter analyze and fixed any errors.

[greg-tekai-com]

Wow, great turnaround time!

Quick question as I'm not sure. By adding the addOnFailureListener, should the addOnCompleteListener above it be changed to addOnSuccessListener so that it doesn't still catch failures, or will only the failure listener fire?

I'm not that familiar with the code, but based on the docs, I was expecting addOnCompleteListener was like a finally block. If not, please ignore me.

Thanks!

[jozefvodicka]

Thanks so much for proposed solution, is it possible to finalise it so the pull request can be created?

[tentenponce]

hi @TimHoogstrate the crash points to LocationSettingsResponse lsr = response.getResult(); on FusedLocationClient line 162. The failure handling is applied on LocationServices.getSettingsClient(context).checkLocationSettings(new LocationSettingsRequest.Builder().build()) which seems to be successful since we are reaching the response.getResult() and encountering the crash from there.

I don't think the solution will fix the issue.

attaching also another similar issue with react native, and their approach resolves the issue. However, still it doesn't fix the root cause since they just added a simple try catch. This probably avoid crashes, while waiting for Google to fix it.
https://github.com/Agontuk/react-native-geolocation-service/pull/147/files

[greg-tekai-com]

The problem is it still runs the code after the if (!response.isSuccessful()) block on failures, leading to checking the result regardless. It should exit the method after calling listener.onLocationServiceError in that block if it was a failure, or wrap the rest in an else block.

If we add the addOnFailureListener as per the PR, then it also needs to change the addOnCompleteListener to addOnSuccessListener, removing the check as it should be redundant, or leave it, but ensure we don't call listener.onLocationServiceError again if it was a failure. Calling it twice returns a result on the method channel twice and that will also fail.

Not sure we'll use it, but the new error code is nice addition so that we can determine it failed to check, rather than default to disabled.

[kazbek-sultanov-t]

We are encountering exactly the same error.
@TimHoogstrate is there plan to merge these changes?
Or merge the proposed PR by @tentenponce ?