-
Notifications
You must be signed in to change notification settings - Fork 283
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
BUG: App is not downloading JSON data from configured Trusted Sources DHM_URGENT #430
Comments
Thanks for filing this. Should be fixed in the next beta drop which will come out either tonight or tomorrow I think. Could use your help in verifying. Just so we have the reference, @tremblerz is fixing a bunch of this code in #378 |
Happy to test when you have a fix. Looking forward to finally seeing a big red X on my phone! |
@kenpugsley v0.9.2 still not available as far as I can see. Any idea when we'll jave something to test here? |
I believe we are on track here, but I think it's pretty important we fix before we push to the App Store. |
@kenpugsley With v0.9.2 I am not seeing any different behaviour here. Even with local data from a URL like this, I am still not getting a match http://diarmidmackenzie.pythonanywhere.com/infection-data?longitude=42&latitude=13 Were you expecting this to be fixed in v0.92, or are there still known issues? |
We'll have to replicate this issue to diagnose. I did expect that the issue would be resolved in 0.9.2, so we'll have to see what is going on. Adding the High Priority label. |
OK - I have checked, and I'm still not seeing any request hitting my web server access logs. So we are still not even getting as far as an outbound request. What diagnostics can I provide from the app that would be useful? Can you build a debug build for me to run? |
I addressed several things in #499 . Please recheck either that PR branch or after it is merged. |
At v0.9.4 this is still not working. Still not seeing any HTTP requests in the server access logs. Therefore data is not even being requested (still). Specifically:
My assumption is that there shoudl be an initial JSON data pull immediately that the HA is configured. Is that correct? I suppose it is possible that the data pull is delayed until the appropriate 12h download time... (if so, that's horrible from a testability point of view). |
I do not see this in my local testing. I see the requests hitting my servers. When I come back up for air from #529 I can try and take a look. @tremblerz are you able to look at this sooner than me? |
I am gonna add eula thing for now and will come back on this afterwards |
Is anyone lookign at this? I'm concerned that ken says this was working for him - I'm worried it might have been something about my setup causing problems. Available to collaborate on this in ay way that would help... |
I'm interested to know what the UX is supposed to be if you mistype a URL, or if you have no intenet connetcivity (e.g. airplane mode on) when you set up the HA. It would be greeat to get feedback whether the HA had been contactable (especially for typos). As it is, you just enter any string, and it goes on the list and you never get any feedback whether it was mistyped, whether data is downloading etc. A better UX on various classes of failure might help us to diagnose this more quickly. |
I'd like to treat the UX issues here as a separate issue. I'll try to dig in tonight and see if I can replicate what's going on. I should be able to tell if I'm getting data from your server or not. Will update here.
|
OK... so plenty of egg on face here. As far as I can tell, the app should be downloading data correctly (though I can't explain why you don't see the hits in your logs). When I run the app in both the Android and iOS simulators I can see that the data is indeed being downloaded. However, there are a couple of problems to deal with: Problem 1: Your web service (http://diarmidmackenzie.pythonanywhere.com/infection-data?longitude=42&latitude=13) follows the Safe Places server documentation (https://github.com/tripleblindmarket/safe-places/blob/develop/Safe-Places-Server.md), but it turns out that documentation is incorrect. I just verified with @penrods, that a change was overlooked when the data structure was simplified some time ago. That doc will be fixed shortly. The correct JSON structure that Safe Places produces is shown in the example here: https://github.com/tripleblindmarket/safe-places/blob/develop/examples/safe-paths.json. Please try adjusting your service to follow that structure. Problem 2: Because the app silently fails on any issue when loading health care authority data, the problem was about 100x harder to find. That's something we definitely need to prioritize for the v1 release. I'll talk with the dev team to prioritize adding an error message when the loading of health authority data fails to help diagnose these kinds of problems. Thanks for continuing to push on this, and sorry it's taken so long to look at it in enough detail to figure out the problem. Please update on how it goes after adjusting your endpoint. I'd like to keep this issue open to track the need for better UX on data load failures. |
OK - server updated to new format. I hope this is now correct? http://diarmidmackenzie.pythonanywhere.com/infection-data?longitude=1&latitude=2 However I still don't seem to be able to reach it (and still not seeing any access logs). Does this now work with your app & my server? Some questions & requests:
Both these would hugely help observability (and hence overall testability). |
Yes, it appears to work for me now. Structure looks correct, and I'm able to use it to cause a the "you may be exposed" to happen for me with my location.
This I 'm still puzzled by. If you've not tried this already, can you do a share of your export history and open the file. Take the most recent lat/lon, and use that in the url for your test health authority. Make sure you match to least 4 decimal points. I think that should cause you to get an "exposed" state to trigger.
Yes, you should include the
Agree with all of that. Will be talking to the dev team on what we should do here. I know having basic info would help not only people testing the app, but technical end users as well. Ideally we should also have a way for the user to know more easily that a health authority hasn't been reached, but not sure if that would make v1.0 or not. |
OK - one problem I was having is that the prefix has to be https:// Not http:// My phone kept auto-correcting https:// to one of the last 2. I don't think that's been the only problem - but it's been the most recent one. Now all working! |
@kenpugsley Do you want to keep this open to track all the UX issues we spotted? Or shall we close & spin up more to cover. Key points:
|
@diarmidmackenzie I think we can close this out. Thoughts? |
Hi - just saw this update. Even if the "add authority by URL" function won't be used much in the field, it's useful in test and it very hard to use due to the flaws above. All the following would be very valueable:
Can we raise a Jira to cover these? |
I had been using the two sample HAs, with no indications of success from the app. I assumed that was because the GPS data didn't match my GPS location.
Later, I set up my own test HA hosted in Google Cloud, with geographically local data. On the "Choose health authority" page, I entered "Add authority by URL" then I entered the URL where my JSON data is hosted: https://safe-paths-273819.ew.r.appspot.com/infected.json
At this point, I expected the App to contact the server and pull the JSON data.
Since I have access to the server, I can check access logs, and I don't see the app even requesting the JSON data from my server, let alone validating it.
Discussed on Slack with Abhishek here, and he said he thought this was due to a bug that has already been identified in this area. Someone (I don't know who) is working on a fix.
https://covidsafepaths.slack.com/archives/C010RMS2EE9/p1586552412184200
Ken asked me to raise an issue to ensure this was tracked.
I'd love to add some diags or similar, but not sure what would be helpful.
The text was updated successfully, but these errors were encountered: