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

add another error to the isConnRefused check #1951

Merged
merged 1 commit into from
Nov 9, 2015
Merged

add another error to the isConnRefused check #1951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions cmd/ipfs/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"io"
"math/rand"
"net"
"net/url"
"os"
"os/signal"
"runtime"
Expand Down Expand Up @@ -671,6 +673,15 @@ func apiClientForAddr(addr ma.Multiaddr) (cmdsHttp.Client, error) {
}

func isConnRefused(err error) bool {
return strings.Contains(err.Error(), "connection refused") ||
strings.Contains(err.Error(), "target machine actively refused it")
// unwrap url errors from http calls
if urlerr, ok := err.(*url.Error); ok {
err = urlerr.Err
}

netoperr, ok := err.(*net.OpError)
if !ok {
return false
}

return netoperr.Op == "dial"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not sure this one statement captures the two other errors. also no longer works with a non-url net.OpError. and if a dial fails for some other reason (like memory alloc failure or some other random happenstance) this will not discern (may be ok).

how about something like:

func isConnRefused(err error) bool {
  switch e := err.(type) {
  case *url.Error:
    return isConnRefused(e.Err)
  case *net.OpError:
    return e.Op == "dial" && (further condition that e really means failure to connect here)
  default:
    return false
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're not going to get a netOp error from a call to http.Post though. The net.OpError is always wrapped before being returned down to us.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then rename it isHTTPConnRefused(...) if you dont want people to use it for other net conn error checking

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(id just support both probably)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, but there are several strings.Contains err checks elsewhere in the code that should be fixed.

}
34 changes: 21 additions & 13 deletions repo/fsrepo/fsrepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"strconv"
"strings"
"sync"
Expand All @@ -26,8 +26,11 @@ import (
u "github.com/ipfs/go-ipfs/util"
util "github.com/ipfs/go-ipfs/util"
ds2 "github.com/ipfs/go-ipfs/util/datastore2"
logging "github.com/ipfs/go-ipfs/vendor/QmQg1J6vikuXF9oDvm4wpdeAUvvkVEKW1EYDw9HhTMnP2b/go-log"
)

var log = logging.Logger("fsrepo")

// version number that we are currently expecting to see
var RepoVersion = "2"

Expand Down Expand Up @@ -163,7 +166,7 @@ func open(repoPath string) (repo.Repo, error) {
}

func newFSRepo(rpath string) (*FSRepo, error) {
expPath, err := u.TildeExpansion(path.Clean(rpath))
expPath, err := u.TildeExpansion(filepath.Clean(rpath))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -247,17 +250,17 @@ func Init(repoPath string, conf *config.Config) error {

// The actual datastore contents are initialized lazily when Opened.
// During Init, we merely check that the directory is writeable.
leveldbPath := path.Join(repoPath, leveldbDirectory)
leveldbPath := filepath.Join(repoPath, leveldbDirectory)
if err := dir.Writable(leveldbPath); err != nil {
return fmt.Errorf("datastore: %s", err)
}

flatfsPath := path.Join(repoPath, flatfsDirectory)
flatfsPath := filepath.Join(repoPath, flatfsDirectory)
if err := dir.Writable(flatfsPath); err != nil {
return fmt.Errorf("datastore: %s", err)
}

if err := dir.Writable(path.Join(repoPath, "logs")); err != nil {
if err := dir.Writable(filepath.Join(repoPath, "logs")); err != nil {
return err
}

Expand All @@ -270,14 +273,14 @@ func Init(repoPath string, conf *config.Config) error {

// Remove recursively removes the FSRepo at |path|.
func Remove(repoPath string) error {
repoPath = path.Clean(repoPath)
repoPath = filepath.Clean(repoPath)
return os.RemoveAll(repoPath)
}

// LockedByOtherProcess returns true if the FSRepo is locked by another
// process. If true, then the repo cannot be opened by this process.
func LockedByOtherProcess(repoPath string) (bool, error) {
repoPath = path.Clean(repoPath)
repoPath = filepath.Clean(repoPath)
// NB: the lock is only held when repos are Open
return lockfile.Locked(repoPath)
}
Expand All @@ -287,8 +290,8 @@ func LockedByOtherProcess(repoPath string) (bool, error) {
// process may read this file. modifying this file, therefore, should
// use "mv" to replace the whole file and avoid interleaved read/writes.
func APIAddr(repoPath string) (string, error) {
repoPath = path.Clean(repoPath)
apiFilePath := path.Join(repoPath, apiFile)
repoPath = filepath.Clean(repoPath)
apiFilePath := filepath.Join(repoPath, apiFile)

// if there is no file, assume there is no api addr.
f, err := os.Open(apiFilePath)
Expand All @@ -315,7 +318,7 @@ func APIAddr(repoPath string) (string, error) {

// SetAPIAddr writes the API Addr to the /api file.
func (r *FSRepo) SetAPIAddr(addr string) error {
f, err := os.Create(path.Join(r.path, apiFile))
f, err := os.Create(filepath.Join(r.path, apiFile))
if err != nil {
return err
}
Expand All @@ -341,7 +344,7 @@ func (r *FSRepo) openConfig() error {

// openDatastore returns an error if the config file is not present.
func (r *FSRepo) openDatastore() error {
leveldbPath := path.Join(r.path, leveldbDirectory)
leveldbPath := filepath.Join(r.path, leveldbDirectory)
var err error
// save leveldb reference so it can be neatly closed afterward
leveldbDS, err := levelds.NewDatastore(leveldbPath, &levelds.Options{
Expand All @@ -359,7 +362,7 @@ func (r *FSRepo) openDatastore() error {
// including "/" from datastore.Key and 2 bytes from multihash. To
// reach a uniform 256-way split, we need approximately 4 bytes of
// prefix.
blocksDS, err := flatfs.New(path.Join(r.path, flatfsDirectory), 4)
blocksDS, err := flatfs.New(filepath.Join(r.path, flatfsDirectory), 4)
if err != nil {
return errors.New("unable to open flatfs datastore")
}
Expand Down Expand Up @@ -410,6 +413,11 @@ func (r *FSRepo) Close() error {
return err
}

err := os.Remove(filepath.Join(r.path, apiFile))
if err != nil {
log.Warning("error removing api file: ", err)
}

// This code existed in the previous versions, but
// EventlogComponent.Close was never called. Preserving here
// pending further discussion.
Expand Down Expand Up @@ -600,7 +608,7 @@ func isInitializedUnsynced(repoPath string) bool {
if !configIsInitialized(repoPath) {
return false
}
if !util.FileExists(path.Join(repoPath, leveldbDirectory)) {
if !util.FileExists(filepath.Join(repoPath, leveldbDirectory)) {
return false
}
return true
Expand Down