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.

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

net: document potential values for the "Op" in net.OpError #36703

Open
kevinburke1 opened this issue Jan 23, 2020 · 11 comments
Open

net: document potential values for the "Op" in net.OpError #36703

kevinburke1 opened this issue Jan 23, 2020 · 11 comments

Comments

@kevinburke1
Copy link

@kevinburke1 kevinburke1 commented Jan 23, 2020

The current documentation reads:

// OpError is the error type usually returned by functions in the 
// net package. It describes the operation, network type, and address of an error.

type OpError struct {
    // Op is the operation which caused the error, such as
    // "read" or "write".
    Op string

As far as I am aware the only way to determine which "Op"'s exist is by grepping the source code of the net package.

In particular I am interested in determining whether an error with an Op value of "dial" could have ever been returned after a client sent bytes of a request to a remote HTTP server.

@kevinburke1
Copy link
Author

@kevinburke1 kevinburke1 commented Jan 23, 2020

Here's the list of Op values that I can find in the net package, assuming that nothing outside of src/net generates a *net.OpError.

$ ag 'Op:' src/net | egrep -o 'Op: "([^"]+)"' | sort | uniq
Op: "accept"
Op: "addmulti"
Op: "announce"
Op: "close"
Op: "dial"
Op: "file"
Op: "listen"
Op: "op"
Op: "proxyconnect"
Op: "raw-control"
Op: "raw-read"
Op: "raw-write"
Op: "read"
Op: "readfrom"
Op: "route"
Op: "set"
Op: "write"
Op: "writev"
Op: "wsasend"

@toothrot toothrot added this to the Backlog milestone Jan 23, 2020
@toothrot
Copy link
Contributor

@toothrot toothrot commented Jan 23, 2020

Great question, @kevinburkemeter
/cc @mikioh @bradfitz @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 23, 2020

I think that before listing the various options we should rationalize them. They seem to be a mix of operations like "dial" and system calls like "wsasend". What are we trying to tell the programmer? What do we expect them to do with this information?

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 19, 2020

Change https://golang.org/cl/263557 mentions this issue: net: document where OpError.Op values can be found

@kevinburke1
Copy link
Author

@kevinburke1 kevinburke1 commented Oct 19, 2020

@ianlancetaylor I sent a CL; I figure at the very least we could document the current state of things; if they improve we can document the improvements.

My motivation or rationalization was trying to figure out what part of the dial operation actually sent bytes to the remote server and whether I could determine what stage of the operation we were in based on the Op value. In theory I could use that to, for example, determine with 100% confidence that a Dial operation could be retried because the remote server did not receive any bytes of the request.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 19, 2020

My concern is that once we document the set of Op values in a release, we are locked into that set and can't really change it.

I would be fine with listing the various Op values in a comment that is not part of the package docs.

@kevinburke1
Copy link
Author

@kevinburke1 kevinburke1 commented Oct 21, 2020

Could I link to this Github issue? Or to a page on the wiki? Are those temporary enough?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 21, 2020

I'm really sorry, now I've lost track of what we are talking about.

I'm saying: before we officially document the list of values, let's rationalize them. Because right now they don't make a lot of sense to me (what is "announce"?).

@networkimprov
Copy link

@networkimprov networkimprov commented Oct 22, 2020

Could you clarify what you mean by "rationalize"? Aren't the Ops references to net APIs?

The current values are part of the OpError API, even if not documented, so you can't change them. Why would you not document them?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 22, 2020

As far as I know there is no net API named proxyconnect, but according to the comment above there is an OpError value with Op set to "proxyconnect".

If the values are not documented, then by definition they are not part of the API. Now, admittedly, they are part of the way that the current code works, and it is possible that changing them would break existing code. But I think that is fairly unlikely. At least, I think it is unlikely enough that we can at least consider changing the values where it makes sense.

@networkimprov
Copy link

@networkimprov networkimprov commented Oct 22, 2020

Returning the same constants in an exported type's field for 10y looks like API to me :-)

But sure, rename "proxyconnect" to "dial:proxy" or somesuch; maybe no one will complain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants