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

Gracefully handle SocketTimeout when MLS not available #794

Merged
merged 1 commit into from Jul 6, 2023

Conversation

MikeRatcliffe
Copy link
Contributor

Fixes #684

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Looking good, @MikeRatcliffe have you found a reliable way to reproduce it?

I hit that occasionally but now that I try to reproduce it I'm unable to. I tried disabling wifi, blocking DNS resolution on my router, shutting down the connection to my ISP... with no luck.

The backtrace is pretty clear anyway, but I just wanted to check whether simply managing the exception is enough, there might be other side effects, like what happens if we try to do a search after the location service failed...

@MikeRatcliffe
Copy link
Contributor Author

@svillar

You are right. After some testing it appears that geolocation doesn't work even with the release version in the app store... I'll log a separate bug blocking this.

I tested using https://www.audero.it/demo/geolocation-api-demo.html

@svillar
Copy link
Member

svillar commented Jul 5, 2023

@MikeRatcliffe you have to give geolocation permissions to the app to get it working. I think that works fine.

Anyway my question was whether you have a reliable way to reproduce it, and check that it indeed does not add any side behaviour. Although it couldn't be worst than crashing...

@svillar
Copy link
Member

svillar commented Jul 5, 2023

Yeah, so the key is that Meta does not allow VR apps to ask for location permissions. So this cannot be reproduced in Meta devices. I'll try in Pico and others where it should be easy to reproduce.

@MikeRatcliffe
Copy link
Contributor Author

Looking good, @MikeRatcliffe have you found a reliable way to reproduce it?

No, I couldn't find a way to replicate it. I chose to manually throw a SocketTimeoutException and made sure it was dealt with in the try / catch block.

With this patch, Wolvic starts up instead of throwing and bailing.

I didn't find any problems using search etc. when testing this.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Thanks!

@svillar svillar merged commit 24e7eb4 into Igalia:main Jul 6, 2023
0 of 7 checks passed
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.

Crash when the location service is not able to locate a provider
2 participants