refactor(rpc): better error messages on RPC EOF (#1229)

* refactor(rpc): better error messages on RPC EOF

* refactor(rpc): better language and nil error chacking

lib/config.go

@@ -34,7 +34,7 @@ type GetConfigParams struct {
 // as a slice of bytes the bytes can be formatted as json, concise json, or yaml
 func (m *ConfigMethods) GetConfig(p *GetConfigParams, res *[]byte) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("ConfigMethods.GetConfig", p, res)
+		return checkRPCError(m.inst.rpc.Call("ConfigMethods.GetConfig", p, res))
 	}
 
 	var (
@@ -77,7 +77,7 @@ func (m *ConfigMethods) GetConfig(p *GetConfigParams, res *[]byte) (err error) {
 // SetConfig validates, updates and saves the config
 func (m *ConfigMethods) SetConfig(update *config.Config, set *bool) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("ConfigMethods.SetConfig", update, set)
+		return checkRPCError(m.inst.rpc.Call("ConfigMethods.SetConfig", update, set))
 	}
 
 	if err = update.Validate(); err != nil {

lib/datasets.go

@@ -68,7 +68,7 @@ func NewDatasetRequestsInstance(inst *Instance) *DatasetRequests {
 func (r *DatasetRequests) List(p *ListParams, res *[]dsref.VersionInfo) error {
 	if r.cli != nil {
 		p.RPC = true
-		return r.cli.Call("DatasetRequests.List", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.List", p, res))
 	}
 	ctx := context.TODO()
 
@@ -185,7 +185,7 @@ func (r *DatasetRequests) List(p *ListParams, res *[]dsref.VersionInfo) error {
 // ListRawRefs gets the list of raw references as string
 func (r *DatasetRequests) ListRawRefs(p *ListParams, text *string) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.ListRawRefs", p, text)
+		return checkRPCError(r.cli.Call("DatasetRequests.ListRawRefs", p, text))
 	}
 	ctx := context.TODO()
 	if p.UseDscache {
@@ -232,7 +232,7 @@ type GetResult struct {
 // with the generated stats.
 func (r *DatasetRequests) Get(p *GetParams, res *GetResult) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Get", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Get", p, res))
 	}
 	ctx := context.TODO()
 
@@ -442,7 +442,7 @@ func (p *SaveParams) AbsolutizePaths() error {
 func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err error) {
 	if r.cli != nil {
 		p.ScriptOutput = nil
-		return r.cli.Call("DatasetRequests.Save", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Save", p, res))
 	}
 	ctx := context.TODO()
 
@@ -657,7 +657,7 @@ type SetPublishStatusParams struct {
 // SetPublishStatus updates the publicity of a reference in the peer's namespace
 func (r *DatasetRequests) SetPublishStatus(p *SetPublishStatusParams, publishedRef *reporef.DatasetRef) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.SetPublishStatus", p, publishedRef)
+		return checkRPCError(r.cli.Call("DatasetRequests.SetPublishStatus", p, publishedRef))
 	}
 
 	ref, err := repo.ParseDatasetRef(p.Ref)
@@ -685,7 +685,7 @@ type RenameParams struct {
 // Rename changes a user's given name for a dataset
 func (r *DatasetRequests) Rename(p *RenameParams, res *dsref.VersionInfo) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Rename", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Rename", p, res))
 	}
 	ctx := context.TODO()
 
@@ -748,7 +748,7 @@ var ErrCantRemoveDirectoryDirty = fmt.Errorf("cannot remove files while working
 // Remove a dataset entirely or remove a certain number of revisions
 func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Remove", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Remove", p, res))
 	}
 	ctx := context.TODO()
 
@@ -926,7 +926,7 @@ func (r *DatasetRequests) Add(p *AddParams, res *reporef.DatasetRef) (err error)
 	}
 
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Add", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Add", p, res))
 	}
 	ctx := context.TODO()
 
@@ -979,7 +979,7 @@ type ValidateDatasetParams struct {
 // Validate gives a dataset of errors and issues for a given dataset
 func (r *DatasetRequests) Validate(p *ValidateDatasetParams, valerrs *[]jsonschema.ValError) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Validate", p, valerrs)
+		return checkRPCError(r.cli.Call("DatasetRequests.Validate", p, valerrs))
 	}
 	ctx := context.TODO()
 
@@ -1101,7 +1101,7 @@ func (r *DatasetRequests) Validate(p *ValidateDatasetParams, valerrs *[]jsonsche
 // Manifest generates a manifest for a dataset path
 func (r *DatasetRequests) Manifest(refstr *string, m *dag.Manifest) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Manifest", refstr, m)
+		return checkRPCError(r.cli.Call("DatasetRequests.Manifest", refstr, m))
 	}
 	ctx := context.TODO()
 
@@ -1125,7 +1125,7 @@ func (r *DatasetRequests) Manifest(refstr *string, m *dag.Manifest) (err error)
 // ManifestMissing generates a manifest of blocks that are not present on this repo for a given manifest
 func (r *DatasetRequests) ManifestMissing(a, b *dag.Manifest) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Manifest", a, b)
+		return checkRPCError(r.cli.Call("DatasetRequests.Manifest", a, b))
 	}
 	ctx := context.TODO()
 
@@ -1146,7 +1146,7 @@ type DAGInfoParams struct {
 // DAGInfo generates a dag.Info for a dataset path. If a label is given, DAGInfo will generate a sub-dag.Info at that label.
 func (r *DatasetRequests) DAGInfo(s *DAGInfoParams, i *dag.Info) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.DAGInfo", s, i)
+		return checkRPCError(r.cli.Call("DatasetRequests.DAGInfo", s, i))
 	}
 	ctx := context.TODO()
 
@@ -1184,7 +1184,7 @@ type StatsResponse struct {
 // Stats generates stats for a dataset
 func (r *DatasetRequests) Stats(p *StatsParams, res *StatsResponse) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Stats", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Stats", p, res))
 	}
 	ctx := context.TODO()
 	if p.Dataset == nil {

lib/diff.go

@@ -60,7 +60,7 @@ func (r *DatasetRequests) Diff(p *DiffParams, res *DiffResponse) (err error) {
 	}
 
 	if r.cli != nil {
-		return r.cli.Call("DatasetRequests.Diff", p, res)
+		return checkRPCError(r.cli.Call("DatasetRequests.Diff", p, res))
 	}
 	ctx := context.TODO()
 

lib/export.go

@@ -64,7 +64,7 @@ func (r *ExportRequests) Export(p *ExportParams, fileWritten *string) (err error
 	}
 
 	if r.cli != nil {
-		return r.cli.Call("ExportRequests.Export", p, fileWritten)
+		return checkRPCError(r.cli.Call("ExportRequests.Export", p, fileWritten))
 	}
 	ctx := context.TODO()
 

lib/fsi.go

@@ -32,7 +32,7 @@ func (m FSIMethods) CoreRequestsName() string { return "fsi" }
 // LinkedRefs lists all fsi links
 func (m *FSIMethods) LinkedRefs(p *ListParams, res *[]reporef.DatasetRef) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.LinkedRefs", p, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.LinkedRefs", p, res))
 	}
 
 	*res, err = m.inst.fsi.LinkedRefs(p.Offset, p.Limit)
@@ -55,7 +55,7 @@ func (m *FSIMethods) CreateLink(p *LinkParams, res *string) (err error) {
 	p.Dir = path
 
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.CreateLink", p, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.CreateLink", p, res))
 	}
 
 	*res, _, err = m.inst.fsi.CreateLink(p.Dir, p.Ref)
@@ -67,7 +67,7 @@ func (m *FSIMethods) CreateLink(p *LinkParams, res *string) (err error) {
 // will remove the fsi path from that reference, and remove the link file from that fsi path
 func (m *FSIMethods) Unlink(p *LinkParams, res *string) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.Unlink", p, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.Unlink", p, res))
 	}
 
 	if p.Dir != "" && p.Ref != "" {
@@ -108,7 +108,7 @@ type StatusItem = fsi.StatusItem
 // version in the repo. Must only be called if FSI is enabled for this dataset.
 func (m *FSIMethods) Status(dir *string, res *[]StatusItem) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.Status", dir, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.Status", dir, res))
 	}
 	ctx := context.TODO()
 
@@ -121,7 +121,7 @@ func (m *FSIMethods) Status(dir *string, res *[]StatusItem) (err error) {
 // is not linked.
 func (m *FSIMethods) StatusForAlias(alias *string, res *[]StatusItem) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.AliasStatus", alias, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.AliasStatus", alias, res))
 	}
 	ctx := context.TODO()
 
@@ -137,7 +137,7 @@ func (m *FSIMethods) StatusForAlias(alias *string, res *[]StatusItem) (err error
 // dataset reference. Not used for FSI.
 func (m *FSIMethods) WhatChanged(ref *string, res *[]StatusItem) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.WhatChanged", ref, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.WhatChanged", ref, res))
 	}
 	ctx := context.TODO()
 
@@ -154,7 +154,7 @@ type CheckoutParams struct {
 // Checkout method writes a dataset to a directory as individual files.
 func (m *FSIMethods) Checkout(p *CheckoutParams, out *string) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.Checkout", p, out)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.Checkout", p, out))
 	}
 	ctx := context.TODO()
 
@@ -231,7 +231,7 @@ type FSIWriteParams struct {
 // Write mutates a linked dataset on the filesystem
 func (m *FSIMethods) Write(p *FSIWriteParams, res *[]StatusItem) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.Write", p, res)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.Write", p, res))
 	}
 	ctx := context.TODO()
 
@@ -274,7 +274,7 @@ type RestoreParams struct {
 // Restore method restores a component or all of the component files of a dataset from the repo
 func (m *FSIMethods) Restore(p *RestoreParams, out *string) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.Restore", p, out)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.Restore", p, out))
 	}
 	ctx := context.TODO()
 
@@ -344,7 +344,7 @@ type InitFSIDatasetParams = fsi.InitParams
 // InitDataset creates a new dataset and FSI link
 func (m *FSIMethods) InitDataset(p *InitFSIDatasetParams, name *string) (err error) {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.InitDataset", p, name)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.InitDataset", p, name))
 	}
 
 	*name, err = m.inst.fsi.InitDataset(*p)
@@ -360,7 +360,7 @@ type EnsureParams struct {
 // EnsureRef will modify the directory path in the repo for the given reference
 func (m *FSIMethods) EnsureRef(p *EnsureParams, out *bool) error {
 	if m.inst.rpc != nil {
-		return m.inst.rpc.Call("FSIMethods.EnsureRef", p, out)
+		return checkRPCError(m.inst.rpc.Call("FSIMethods.EnsureRef", p, out))
 	}
 
 	return m.inst.fsi.ModifyLinkDirectory(p.Dir, p.Ref)

lib/lib.go

@@ -23,6 +23,7 @@ import (
 	"github.com/qri-io/qri/config"
 	"github.com/qri-io/qri/config/migrate"
 	"github.com/qri-io/qri/dscache"
+	qrierr "github.com/qri-io/qri/errors"
 	"github.com/qri-io/qri/event"
 	"github.com/qri-io/qri/fsi"
 	"github.com/qri-io/qri/logbook"
@@ -743,3 +744,23 @@ func (inst *Instance) RemoteClient() remote.Client {
 func (inst *Instance) Teardown() {
 	inst.teardown()
 }
+
+// checkRPCError validates RPC errors and in case of EOF returns a
+// more user friendly message
+func checkRPCError(err error) error {
+	if err == nil {
+		return nil
+	}
+	if strings.Contains(err.Error(), "EOF") {
+		msg := `Qri couldn't parse the response and is unsure if it was successful. 
+It is possible you have a Qri node running or the Desktop app is open.
+Try closing them and running the command again.
+Check our issue tracker for RPC issues & feature requests:
+  https://github.com/qri-io/qri/issues?q=is:issue+label:RPC
+
+Error:
+%s`
+		return qrierr.New(err, fmt.Sprintf(msg, err.Error()))
+	}
+	return err
+}

lib/log.go

@@ -48,7 +48,7 @@ type DatasetLogItem = logbook.DatasetLogItem
 // Log returns the history of changes for a given dataset
 func (r *LogRequests) Log(params *LogParams, res *[]DatasetLogItem) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("LogRequests.Log", params, res)
+		return checkRPCError(r.cli.Call("LogRequests.Log", params, res))
 	}
 	ctx := context.TODO()
 
@@ -94,7 +94,7 @@ type LogEntry = logbook.LogEntry
 // Logbook lists log entries for actions taken on a given dataset
 func (r *LogRequests) Logbook(p *RefListParams, res *[]LogEntry) error {
 	if r.cli != nil {
-		return r.cli.Call("LogRequests.Logbook", p, res)
+		return checkRPCError(r.cli.Call("LogRequests.Logbook", p, res))
 	}
 	ctx := context.TODO()
 
@@ -122,7 +122,7 @@ type PlainLogs = []logbook.PlainLog
 // PlainLogs encodes the full logbook as human-oriented json
 func (r *LogRequests) PlainLogs(p *PlainLogsParams, res *PlainLogs) (err error) {
 	if r.cli != nil {
-		return r.cli.Call("LogRequests.PlainLogs", p, res)
+		return checkRPCError(r.cli.Call("LogRequests.PlainLogs", p, res))
 	}
 	ctx := context.TODO()
 	*res, err = r.node.Repo.Logbook().PlainLogs(ctx)

lib/peers.go

@@ -51,7 +51,7 @@ type PeerListParams struct {
 // List lists Peers on the qri network
 func (d *PeerRequests) List(p *PeerListParams, res *[]*config.ProfilePod) (err error) {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.List", p, res)
+		return checkRPCError(d.cli.Call("PeerRequests.List", p, res))
 	}
 	if d.qriNode == nil {
 		return fmt.Errorf("error: not connected, run `qri connect` in another window")
@@ -69,7 +69,7 @@ func (d *PeerRequests) List(p *PeerListParams, res *[]*config.ProfilePod) (err e
 // IPFS this will also return connected IPFS nodes
 func (d *PeerRequests) ConnectedIPFSPeers(limit *int, peers *[]string) error {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.ConnectedIPFSPeers", limit, peers)
+		return checkRPCError(d.cli.Call("PeerRequests.ConnectedIPFSPeers", limit, peers))
 	}
 
 	*peers = d.qriNode.ConnectedPeers()
@@ -79,7 +79,7 @@ func (d *PeerRequests) ConnectedIPFSPeers(limit *int, peers *[]string) error {
 // ConnectedQriProfiles lists profiles we're currently connected to
 func (d *PeerRequests) ConnectedQriProfiles(limit *int, peers *[]*config.ProfilePod) (err error) {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.ConnectedQriProfiles", limit, peers)
+		return checkRPCError(d.cli.Call("PeerRequests.ConnectedQriProfiles", limit, peers))
 	}
 
 	connected := d.qriNode.ConnectedQriProfiles()
@@ -156,7 +156,7 @@ func (p PeerConnectionParamsPod) Decode() (cp p2p.PeerConnectionParams, err erro
 // ConnectToPeer attempts to create a connection with a peer for a given peer.ID
 func (d *PeerRequests) ConnectToPeer(p *PeerConnectionParamsPod, res *config.ProfilePod) error {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.ConnectToPeer", p, res)
+		return checkRPCError(d.cli.Call("PeerRequests.ConnectToPeer", p, res))
 	}
 
 	pcp, err := p.Decode()
@@ -181,7 +181,7 @@ func (d *PeerRequests) ConnectToPeer(p *PeerConnectionParamsPod, res *config.Pro
 // DisconnectFromPeer explicitly closes a peer connection
 func (d *PeerRequests) DisconnectFromPeer(p *PeerConnectionParamsPod, res *bool) error {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.DisconnectFromPeer", p, res)
+		return checkRPCError(d.cli.Call("PeerRequests.DisconnectFromPeer", p, res))
 	}
 
 	pcp, err := p.Decode()
@@ -208,7 +208,7 @@ type PeerInfoParams struct {
 // Info shows peer profile details
 func (d *PeerRequests) Info(p *PeerInfoParams, res *config.ProfilePod) error {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.Info", p, res)
+		return checkRPCError(d.cli.Call("PeerRequests.Info", p, res))
 	}
 
 	// TODO: Move most / all of this to p2p package, perhaps.
@@ -262,7 +262,7 @@ type PeerRefsParams struct {
 // GetReferences lists a peer's named datasets
 func (d *PeerRequests) GetReferences(p *PeerRefsParams, res *[]reporef.DatasetRef) error {
 	if d.cli != nil {
-		return d.cli.Call("PeerRequests.GetReferences", p, res)
+		return checkRPCError(d.cli.Call("PeerRequests.GetReferences", p, res))
 	}
 	ctx := context.TODO()