-
Notifications
You must be signed in to change notification settings - Fork 29
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
Request authorization Always doesn't return after being changed in Settings #18
Comments
Oh geez. This stuff is not particularly testable, so we have no tests. Sorry. |
Yeah I can see why this would happen, though I'm surprised it didn't happen before. |
Not a regression, Apple don’t fire any So… not sure if we can even detect this. |
We can probably listen for the became foreground system notification, but this is a hack. |
I guess our only option is to reject the promise when attempting to upgrade and output a warning to the console telling the user to upgrade manually with a link to this ticket. |
Oh dear, you are indeed correct. I think this is worthy of a TSI. |
So I created a TSI with Apple yesterday, and received a response today:
I agree this feels rather hacky. |
I updated the 'CoreLocationTest' to have an implementation of requesting authorization using application state notifications instead of the location manager delegate. It works, as expected, but also requires a piece of persisted state This probably isn't a suitable solution for PromiseKit though, which is a shame. |
Nice research. My main issue with using the resign active stuff is some other system event could trigger it, eg battery low. So the promise cannot be 100% reliable. Well, at least it seems like we couldn’t be. |
Latest PromiseKit 6.2.6, which kindly had a fix for #16 in it (🙏) seems to have introduced a regression.
The same test app can be used to reproduce the issue, but with a different sequence of events.
Steps:
Expected result:
tada
Actual result:
Also of note, subsequent launches of the app will fail to complete the chain.
The text was updated successfully, but these errors were encountered: