Shell.add, file will be closed by shell.Shell automatically

[treasersimplifies]

I found in request.go:

func (r *Request) Send(c *http.Client) (*Response, error) {
    // snip
    if resp.StatusCode >= http.StatusBadRequest {
        // snip
        io.Copy(ioutil.Discard, resp.Body)
        resp.Body.Close()
    }
}

when developer use go-ipfs-api, they typically:

// var shells []*shell.Shell = ... connection to a lot of ipfs nodes..
localFile, err := os.Open(localfPath)
defer localFile.Close()
if err != nil {
    return nil, err
}
err = retry.Retry(func(attempt uint) error {
    response, err = shells[attempt].Add(localFile)
    if err != nil {
	return err
    }
    return nil
},
    strategy.Limit(limit),
)

So when fail on one shell.add, shell will close the file it pass, it's not good. The close of file should leave to developer.

[welcome[bot]]

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review. In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment. Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

• "Priority" labels will show how urgent this is for the team.
• "Status" labels will show if this is ready to be worked on, blocked, or in progress.
• "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

[Stebalien]

That code closes the response body, not the request file.

However, go-ipfs-files closes the request file when it finishes reading it. This could be changed, but it would be a fair amount of work for little gain (we rely on this logic to close files as we traverse directory trees).

In general, the code above will never work as you'd need to seek back to the beginning of the file for each retry. The current behavior (an error) is significantly better than the bug you'd get if we didn't close the file (empty files).

[aschmahmann]

@treasersimplifies is this issue a request for adding better documentation to the Add command and/or go-ipfs-files? Mutating file handles as you work with them seems pretty common place, but it may be worth pointing out that it's happening.

PRs welcome if you have wording/documentation suggestions here.