StreamAllContext: move to netxlite and add testing
#2654
Assignees
Labels
Comments
bassosimone
added a commit
to ooni/probe-cli
that referenced
this issue
Feb 5, 2024
Reference issue: ooni/probe#2654
bassosimone
added a commit
to ooni/netem
that referenced
this issue
Feb 5, 2024
We need this feature to write test for StreamAllContext, which in turn is one of the TODO referenced by ooni/probe#2654.
bassosimone
added a commit
to ooni/netem
that referenced
this issue
Feb 5, 2024
We need this feature to write test for StreamAllContext, which in turn is one of the TODOs referenced by ooni/probe#2654.
bassosimone
added a commit
to ooni/probe-cli
that referenced
this issue
Feb 5, 2024
This diff moves the `StreamAllContext` implementation from `webconnectivitylte` to `netxlite`. We flagged this diff as BREAKING CHANGE because we also fixed a bug where, in case of context timeout, `StreamAllContext` was suppressing the error rather than reporting it. Re-reading the code, that felt incorrect because it did not allow us to clearly know that we timed out reading the body, which could be caused by throttling. The new behavior seems more accurate. Because of this change, I also felt it was time to add explicit tests for cases where we download fails because it takes too much time for us to fetch the whole response body. I am not 100% sure we are correctly flagging this case, but an `http-failure` probably seems fine at the moment and we can always revisit this as we learn more about throttling. Part of ooni/probe#2654.
bassosimone
changed the title
netxlite and add testing
Murphy-OrangeMud
pushed a commit
to Murphy-OrangeMud/probe-cli
that referenced
this issue
Feb 13, 2024
This diff moves the `StreamAllContext` implementation from `webconnectivitylte` to `netxlite`. We flagged this diff as BREAKING CHANGE because we also fixed a bug where, in case of context timeout, `StreamAllContext` was suppressing the error rather than reporting it. Re-reading the code, that felt incorrect because it did not allow us to clearly know that we timed out reading the body, which could be caused by throttling. The new behavior seems more accurate. Because of this change, I also felt it was time to add explicit tests for cases where we download fails because it takes too much time for us to fetch the whole response body. I am not 100% sure we are correctly flagging this case, but an `http-failure` probably seems fine at the moment and we can always revisit this as we learn more about throttling. Part of ooni/probe#2654.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Implemented in ooni/netem#46 and ooni/probe-cli#1490.
The text was updated successfully, but these errors were encountered: