Skip to content

cmd: validate repo/api file and print nicer error message #3219

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

Merged
merged 1 commit into from
Nov 21, 2016
Merged

cmd: validate repo/api file and print nicer error message #3219

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
2 changes: 1 addition & 1 deletion cmd/ipfs/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func serveHTTPApi(req cmds.Request) (error, <-chan error) {
return fmt.Errorf("serveHTTPApi: ConstructNode() failed: %s", err), nil
}

if err := node.Repo.SetAPIAddr(apiMaddr.String()); err != nil {
if err := node.Repo.SetAPIAddr(apiMaddr); err != nil {
return fmt.Errorf("serveHTTPApi: SetAPIAddr() failed: %s", err), nil
}

Expand Down
58 changes: 43 additions & 15 deletions cmd/ipfs/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package main

import (
"context"
"errors"
"fmt"
"io"
Expand All @@ -16,13 +17,6 @@ import (
"syscall"
"time"

manet "gx/ipfs/QmT6Cp31887FpAc25z25YHgpFJohZedrYLWPPspRtj1Brp/go-multiaddr-net"
ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"

context "context"
logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
u "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"

cmds "github.com/ipfs/go-ipfs/commands"
cmdsCli "github.com/ipfs/go-ipfs/commands/cli"
cmdsHttp "github.com/ipfs/go-ipfs/commands/http"
Expand All @@ -31,7 +25,13 @@ import (
repo "github.com/ipfs/go-ipfs/repo"
config "github.com/ipfs/go-ipfs/repo/config"
fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo"

logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
manet "gx/ipfs/QmT6Cp31887FpAc25z25YHgpFJohZedrYLWPPspRtj1Brp/go-multiaddr-net"
loggables "gx/ipfs/QmTMy4hVSY28DdwJ9kBz6y7q6MuioFzPcpM3Ma3aPjo1i3/go-libp2p-loggables"
ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
osh "gx/ipfs/QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93/go-os-helper"
u "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"
)

// log is the command logger
Expand Down Expand Up @@ -590,23 +590,51 @@ func profileIfEnabled() (func(), error) {
return func() {}, nil
}

var apiFileErrorFmt string = `Failed to parse '%[1]s/api' file.
error: %[2]s
If you're sure go-ipfs isn't running, you can just delete it.
Otherwise check:
`
var checkIPFSUnixFmt = "\tps aux | grep ipfs"
var checkIPFSWinFmt = "\ttasklist | findstr ipfs"

// getApiClient checks the repo, and the given options, checking for
// a running API service. if there is one, it returns a client.
// otherwise, it returns errApiNotRunning, or another error.
func getApiClient(repoPath, apiAddrStr string) (cmdsHttp.Client, error) {
var apiErrorFmt string
switch {
case osh.IsUnix():
apiErrorFmt = apiFileErrorFmt + checkIPFSUnixFmt
case osh.IsWindows():
apiErrorFmt = apiFileErrorFmt + checkIPFSWinFmt
default:
apiErrorFmt = apiFileErrorFmt
}

if apiAddrStr == "" {
var err error
if apiAddrStr, err = fsrepo.APIAddr(repoPath); err != nil {
var addr ma.Multiaddr
var err error
if len(apiAddrStr) != 0 {
addr, err = ma.NewMultiaddr(apiAddrStr)
if err != nil {
return nil, err
}
if len(addr.Protocols()) == 0 {
return nil, fmt.Errorf("mulitaddr doesn't provide any protocols")
}
} else {
addr, err = fsrepo.APIAddr(repoPath)
if err == repo.ErrApiNotRunning {
return nil, err
}
}

addr, err := ma.NewMultiaddr(apiAddrStr)
if err != nil {
return nil, err
if err != nil {
return nil, fmt.Errorf(apiErrorFmt, repoPath, err.Error())
}
}
if len(addr.Protocols()) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Will this ever happen from a passed in apiAddrStr? We don't want to accidentally tell the user about an api file if theyre manually specifying the address

Copy link
Member Author

Choose a reason for hiding this comment

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

This happens when for some reason the API File is empty.

Copy link
Member

Choose a reason for hiding this comment

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

Right, But theres no possible way we can end up with that scenario if the user passes in an api addr with --api right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea that is why I added check in 7512eb9

return nil, fmt.Errorf(apiErrorFmt, repoPath, "multiaddr doesn't provide any protocols")
}

return apiClientForAddr(addr)
}

Expand Down
7 changes: 6 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@
"hash": "QmQfeKxQtBN721pekQh6Jq24adFUjnU65YdY3GNczfuG2T",
"name": "dir-index-html",
"version": "1.0.3"
},
{
"author": "Kubuxu",
"hash": "QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93",
"name": "go-os-helper",
"version": "0.0.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

oh, look at that

Copy link
Member

Choose a reason for hiding this comment

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

what am i looking at?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, ok, not it is good. Just a second ago GH wasn't showing the bracket that is bellow this line.

}
],
"gxVersion": "0.4.0",
Expand All @@ -295,4 +301,3 @@
"name": "go-ipfs",
"version": "0.4.5-dev"
}

16 changes: 9 additions & 7 deletions repo/fsrepo/fsrepo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import (
mfsr "github.com/ipfs/go-ipfs/repo/fsrepo/migrations"
serialize "github.com/ipfs/go-ipfs/repo/fsrepo/serialize"
dir "github.com/ipfs/go-ipfs/thirdparty/dir"

logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
util "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"
"gx/ipfs/QmeqtHtxGfcsfXiou7wqHJARWPKUTUcPdtSfSYYHp48dtQ/go-ds-measure"
)
Expand Down Expand Up @@ -274,17 +276,17 @@ func LockedByOtherProcess(repoPath string) (bool, error) {
// in the fsrepo. This is a concurrent operation, meaning that any
// 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) {
func APIAddr(repoPath string) (ma.Multiaddr, error) {
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)
if err != nil {
if os.IsNotExist(err) {
return "", repo.ErrApiNotRunning
return nil, repo.ErrApiNotRunning
}
return "", err
return nil, err
}
defer f.Close()

Expand All @@ -293,23 +295,23 @@ func APIAddr(repoPath string) (string, error) {
buf := make([]byte, 2048)
n, err := f.Read(buf)
if err != nil && err != io.EOF {
return "", err
return nil, err
}

s := string(buf[:n])
s = strings.TrimSpace(s)
return s, nil
return ma.NewMultiaddr(s)
}

// SetAPIAddr writes the API Addr to the /api file.
func (r *FSRepo) SetAPIAddr(addr string) error {
func (r *FSRepo) SetAPIAddr(addr ma.Multiaddr) error {
f, err := os.Create(filepath.Join(r.path, apiFile))
if err != nil {
return err
}
defer f.Close()

_, err = f.WriteString(addr)
_, err = f.WriteString(addr.String())
return err
}

Expand Down
4 changes: 3 additions & 1 deletion repo/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"errors"

"github.com/ipfs/go-ipfs/repo/config"

ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
)

var errTODO = errors.New("TODO: mock repo")
Expand Down Expand Up @@ -37,4 +39,4 @@ func (m *Mock) GetStorageUsage() (uint64, error) { return 0, nil }

func (m *Mock) Close() error { return errTODO }

func (m *Mock) SetAPIAddr(addr string) error { return errTODO }
func (m *Mock) SetAPIAddr(addr ma.Multiaddr) error { return errTODO }
4 changes: 3 additions & 1 deletion repo/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"io"

config "github.com/ipfs/go-ipfs/repo/config"

ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
ds "gx/ipfs/QmbzuUusHqaLLoNTDEVLcSF6vZDHZDLPC7p4bztRvvkXxU/go-datastore"
)

Expand All @@ -23,7 +25,7 @@ type Repo interface {
GetStorageUsage() (uint64, error)

// SetAPIAddr sets the API address in the repo.
SetAPIAddr(addr string) error
SetAPIAddr(addr ma.Multiaddr) error

io.Closer
}
Expand Down