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

oonidataformat: expose netx failed operation #311

Closed
bassosimone opened this issue Feb 1, 2020 · 0 comments · Fixed by #680
Closed

oonidataformat: expose netx failed operation #311

bassosimone opened this issue Feb 1, 2020 · 0 comments · Fixed by #680
Assignees
@bassosimone
Labels
discuss We need to have a conversation effort/S Small effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine ooni/spec Issues related to github.com/ooni/spec priority/medium Medium priority

Comments

@bassosimone
Copy link
Contributor

I believe it's reasonable to start exposing this information where available. It may be misleading in a bunch of cases, e.g., when we're doing obfs4. However, it seems generally a good thing to have because it enables us to say "we error we did see was when we were doing tls handshake" versus "tcp connect" versus "in the HTTP round trip". This can be a companion field to failure.
@bassosimone bassosimone added enhancement New feature or request priority/medium Medium priority effort/S Small effort ooni/spec Issues related to github.com/ooni/spec ooni/probe-engine Issues related to github.com/ooni/probe-engine discuss We need to have a conversation labels Feb 1, 2020
@bassosimone bassosimone added this to the Sprint 7 - Crown jellyfish milestone Feb 1, 2020
@bassosimone bassosimone self-assigned this Feb 17, 2020
@bassosimone bassosimone removed this from the Sprint 7 - Crown Jellyfish milestone Feb 17, 2020
bassosimone added a commit that referenced this issue Jun 9, 2020
@bassosimone
urlgetter: expose the operation that failed
a939a45 
This is very useful to quickly understand what actually went wrong. It is
a facet of the next design that was never really used.

Also, acknowledge that, in some cases, there isn't a real failed operation
because the error happens before we enter into our network stack.

This happens, for example, when the context we're using is canceled.

To address this potential issue, introduce and use a `"top_level"` operation
that describes this specific situation.

This work is part the telegram rewrite: #646.

Also, with this diff landing we can close #311,
which mentioned oonimkall (the old implementation of netx) but is still
basically talking about the same concept we're implementing here.
@bassosimone bassosimone mentioned this issue Jun 9, 2020
Merged
bassosimone added a commit that referenced this issue Jun 9, 2020
@bassosimone
urlgetter: expose the operation that failed (#680)
746812d 
This is very useful to quickly understand what actually went wrong. It is
a facet of the next design that was never really used.

Also, acknowledge that, in some cases, there isn't a real failed operation
because the error happens before we enter into our network stack.

This happens, for example, when the context we're using is canceled.

To address this potential issue, introduce and use a `"top_level"` operation
that describes this specific situation.

This work is part the telegram rewrite: #646.

Also, with this diff landing we can close #311,
which mentioned oonimkall (the old implementation of netx) but is still
basically talking about the same concept we're implementing here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
discuss We need to have a conversation effort/S Small effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine ooni/spec Issues related to github.com/ooni/spec priority/medium Medium priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

urlgetter: expose the operation that failed ooni/probe-engine
1 participant
@bassosimone