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.

Sign up for GitHub

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

Jump to bottom

webconnectivity: handle down websites such as doh.centraleu.pi-dns.com #2299

Closed
bassosimone opened this issue Sep 13, 2022 · 4 comments · Fixed by ooni/probe-cli#1459
Closed

webconnectivity: handle down websites such as doh.centraleu.pi-dns.com #2299

bassosimone opened this issue Sep 13, 2022 · 4 comments · Fixed by ooni/probe-cli#1459
Assignees
@bassosimone
Labels
2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 bug Something isn't working data quality fixed-by-webconnectivity-lte The next version of webconnectivity fixes this issue methodology issues related to the testing methodology ooni/probe-engine priority/high testing

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Sep 13, 2022 •
edited
Loading

Let us start our journey with the obligatory MAT search:

Screenshot 2022-09-13 at 08 26 17

So, judging from the MAT search, this website presents anomalies. But, is that really the case? Could it be that there is a bug in Web Connectivity v0.4.x that causes the website to erroneously being flagged as anomaly?

Judging from this v0.4.1 measurement maybe we're misflagging the results here. The connections indeed all fail for both the probe and the TH, so this should be flagged as "website down", at least as far as that measurement I showed was concerned. But also this measurement shows similar issues. And also this one. So maybe I have a point here 🥶.

The approach taken by v0.5 LTE for this kind of situations is a bit more correct. In this measurement, it says "look, mate, I don't know", and sets .Blocking to null and .Failure to null, thus making it an errored measurement. However, the best approach in this case would be to recognize that the website is just down and flag is as such like we did for DNS a few commits ago in ooni/probe-cli@b10eea4.
@bassosimone bassosimone added bug Something isn't working testing priority/high methodology issues related to the testing methodology data quality ooni/probe-engine labels Sep 13, 2022
@bassosimone bassosimone self-assigned this Sep 13, 2022
@bassosimone bassosimone mentioned this issue Sep 13, 2022
Merged
@bassosimone
Copy link
Contributor Author

After ooni/probe-cli#957, webconnectivity LTE flags this website as down as it ought to be

@bassosimone bassosimone changed the title webconnectivity@v0.5: handle down websites such as doh.centraleu.pi-dns.com webconnectivity: handle down websites such as doh.centraleu.pi-dns.com Sep 13, 2022
@bassosimone
Copy link
Contributor Author

So, we're fixing this for v0.5 inside ooni/probe-cli#957. It remains to be seen whether we also want to apply a similar fix to the v0.4 implementation, or whether we prefer focusing on v0.5. I'll chat with @hellais to get a second opinion about this topic. I'd ideally like to get more experience with v0.5 and then try to bring some changes back to v0.4 in a batch, because it might be more efficient to do that.

bassosimone added a commit to ooni/probe-cli that referenced this issue Sep 13, 2022
@bassosimone
feat(webconnectivity@v0.5): probe and TH can't connect => website down (
8d8554e 
#957)

This diff introduces a special rule to avoid emitting null, null when all the connects failed in both the probe and the TH.

While there, recognize that the subset of null, null we're hunting actually deals with websites that are down, so change the internal naming to reflect that and make the code easier to read/understand.

See ooni/probe#2299
bassosimone added a commit to ooni/probe-cli that referenced this issue Sep 13, 2022
@bassosimone
[backport] feat(webconnectivity@v0.5): probe and TH can't connect => …
6ca2bca 
…website down (#957)

This diff introduces a special rule to avoid emitting null, null when all the connects failed in both the probe and the TH.

While there, recognize that the subset of null, null we're hunting actually deals with websites that are down, so change the internal naming to reflect that and make the code easier to read/understand.

See ooni/probe#2299
@bassosimone bassosimone added the fixed-by-webconnectivity-lte The next version of webconnectivity fixes this issue label Feb 1, 2023
@bassosimone
Copy link
Contributor Author

I just added the fixed-by-webconnectivity-lte flag such that we can flag all the issues (such as this one), that are open but will automatically be fixed when we switch over to the new implementation of Web Connectivity. I am now going to tag similar issues. Because the development of the new version of Web Connectivity took a long time, we need a way to distinguish between issues that have been fixed in the next version of Web Connectivity and issues that have not been fixed yet.

bassosimone added a commit to ooni/probe-cli that referenced this issue Jan 22, 2024
@bassosimone
fix(minipipeline): make expected TCP, TLS failures WAI
7639bb1 
This diff corrects an embarassing bug in the logic we were using
for expected TCP connect and TLS handshakes failures.

We spotted this issue with the new testcase we added called
websiteDownTCPConnect, which is also included.

Additionally, we needed to fix some tests because the logic bugs
were preventing us from seeing some events.

Part of ooni/probe#2652.

Closes ooni/probe#2299.
@bassosimone
Copy link
Contributor Author

pRr9-V

We're about to push integration tests for this specific test case and close this issue.

@bassosimone bassosimone mentioned this issue Jan 22, 2024
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jan 22, 2024
@bassosimone
minipipeline: fix expected TCP & TLS failures (#1459)
a608f3a 
This diff corrects an embarrassing bugs in the logic we were using for
expected TCP and TLS failures.

We spotted this issue with the new testcase we added called
websiteDownTCPConnect, which is also included.

Additionally, we updated some tests because of the embarrassing bugs we
fixed.

Additionally, we fixed some comments that were outdated.

Part of ooni/probe#2652.

Closes ooni/probe#2299.
@bassosimone bassosimone added the 2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 label Jan 26, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
@bassosimone @Murphy-OrangeMud
minipipeline: fix expected TCP & TLS failures (ooni#1459)
da656b3 
This diff corrects an embarrassing bugs in the logic we were using for
expected TCP and TLS failures.

We spotted this issue with the new testcase we added called
websiteDownTCPConnect, which is also included.

Additionally, we updated some tests because of the embarrassing bugs we
fixed.

Additionally, we fixed some comments that were outdated.

Part of ooni/probe#2652.

Closes ooni/probe#2299.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 bug Something isn't working data quality fixed-by-webconnectivity-lte The next version of webconnectivity fixes this issue methodology issues related to the testing methodology ooni/probe-engine priority/high testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

minipipeline: fix expected TCP & TLS failures ooni/probe-cli
1 participant
@bassosimone