Fix and simplify RPC server error handling.

This change rewrites much of the error handling for the RPC server
components to match a more idiomatic Go error handling style as well as
fix several issues regarding error equality checks.

Closes #94.

account.go

@@ -439,21 +439,19 @@ func (a *Account) Track() {
 	// Request notifications for transactions sending to all wallet
 	// addresses.
 	addrs := a.ActiveAddresses()
-	addrstrs := make([]string, len(addrs))
-	i := 0
+	addrstrs := make([]string, 0, len(addrs))
 	for addr := range addrs {
-		addrstrs[i] = addr.EncodeAddress()
-		i++
+		addrstrs = append(addrstrs, addr.EncodeAddress())
 	}
 
-	jsonErr := NotifyReceived(CurrentServerConn(), addrstrs)
-	if jsonErr != nil {
+	if err := NotifyReceived(CurrentServerConn(), addrstrs); err != nil {
 		log.Error("Unable to request transaction updates for address.")
 	}
 
 	unspent, err := a.TxStore.UnspentOutputs()
 	if err != nil {
 		log.Errorf("Unable to access unspent outputs: %v", err)
+		return
 	}
 	ReqSpentUtxoNtfns(unspent)
 }
@@ -621,11 +619,11 @@ func (a *Account) RecoverAddresses(n int) error {
 
 	// Run a goroutine to rescan blockchain for recovered addresses.
 	go func(addrs []string) {
-		jsonErr := Rescan(CurrentServerConn(), lastInfo.FirstBlock(),
+		err := Rescan(CurrentServerConn(), lastInfo.FirstBlock(),
 			addrs, nil)
-		if jsonErr != nil {
-			log.Errorf("Rescanning for recovered addresses failed: %v",
-				jsonErr.Message)
+		if err != nil {
+			log.Errorf("Rescanning for recovered addresses "+
+				"failed: %v", err)
 		}
 	}(addrStrs)
 
@@ -660,7 +658,10 @@ func ReqSpentUtxoNtfns(credits []*txstore.Credit) {
 		ops = append(ops, op)
 	}
 
-	NotifySpent(CurrentServerConn(), ops)
+	if err := NotifySpent(CurrentServerConn(), ops); err != nil {
+		log.Errorf("Cannot request notifications for spent outputs: %v",
+			err)
+	}
 }
 
 // TotalReceived iterates through an account's transaction history, returning the

acctmgr.go

@@ -473,7 +473,7 @@ func (am *AccountManager) rescanListener() {
 
 		case *RescanFinishedMsg:
 			if e.Error != nil {
-				log.Errorf("Rescan failed: %v", e.Error.Message)
+				log.Errorf("Rescan failed: %v", e.Error)
 				break
 			}
 

cmd.go

@@ -54,11 +54,12 @@ func GetCurBlock() (wallet.BlockStamp, error) {
 		return bs, nil
 	}
 
-	bb, jsonErr := GetBestBlock(CurrentServerConn())
-	if jsonErr != nil {
-		return wallet.BlockStamp{
+	bb, err := GetBestBlock(CurrentServerConn())
+	if err != nil {
+		unknown := wallet.BlockStamp{
 			Height: int32(btcutil.BlockHeightUnknown),
-		}, jsonErr
+		}
+		return unknown, err
 	}
 
 	hash, err := btcwire.NewShaHashFromStr(bb.Hash)
@@ -210,7 +211,7 @@ func main() {
 		// Perform handshake.
 		if err := Handshake(btcd); err != nil {
 			var message string
-			if jsonErr, ok := err.(*btcjson.Error); ok {
+			if jsonErr, ok := err.(btcjson.Error); ok {
 				message = jsonErr.Message
 			} else {
 				message = err.Error()

rescan.go

@@ -17,7 +17,6 @@
 package main
 
 import (
-	"github.com/conformal/btcjson"
 	"github.com/conformal/btcutil"
 	"github.com/conformal/btcwire"
 )
@@ -51,7 +50,7 @@ func (r *RescanProgressMsg) ImplementsRescanMsg() {}
 // possibly-finished rescan, or an error if the rescan failed.
 type RescanFinishedMsg struct {
 	Addresses map[*Account][]btcutil.Address
-	Error     *btcjson.Error
+	Error     error
 }
 
 // ImplementsRescanMsg is implemented to satisify the RescanMsg
@@ -143,7 +142,7 @@ func (b *rescanBatch) merge(job *RescanJob) {
 
 // Status types for the handler.
 type rescanProgress int32
-type rescanFinished *btcjson.Error
+type rescanFinished error
 
 // jobHandler runs the RescanManager's for-select loop to manage rescan jobs
 // and dispatch requests.
@@ -191,7 +190,7 @@ func (m *RescanManager) jobHandler() {
 				if m.msgs != nil {
 					m.msgs <- &RescanFinishedMsg{
 						Addresses: curBatch.addrs,
-						Error:     (*btcjson.Error)(s),
+						Error:     error(s),
 					}
 				}
 				curBatch.done()
@@ -223,8 +222,8 @@ func (m *RescanManager) rpcHandler() {
 		}
 
 		c := CurrentServerConn()
-		jsonErr := Rescan(c, job.StartHeight, addrStrs, job.OutPoints)
-		m.status <- rescanFinished(jsonErr)
+		err := Rescan(c, job.StartHeight, addrStrs, job.OutPoints)
+		m.status <- rescanFinished(err)
 	}
 }
 

rpc.go

@@ -34,13 +34,15 @@ type RawRPCResponse struct {
 // to by the interface rather than using the rules in the encoding/json
 // package to allocate a new variable for the result.  The final result
 // and JSON-RPC error is returned.
-func (r *RawRPCResponse) FinishUnmarshal(v interface{}) (interface{}, *btcjson.Error) {
+//
+// If the returned error is non-nil, it will be a btcjson.Error.
+func (r *RawRPCResponse) FinishUnmarshal(v interface{}) (interface{}, error) {
 	// JSON-RPC spec makes this handling easier-ish because both result and
 	// error cannot be non-nil.
-	var jsonErr *btcjson.Error
 	if r.Error != nil {
+		var jsonErr btcjson.Error
 		if err := json.Unmarshal([]byte(*r.Error), &jsonErr); err != nil {
-			return nil, &btcjson.Error{
+			return nil, btcjson.Error{
 				Code:    btcjson.ErrParse.Code,
 				Message: err.Error(),
 			}
@@ -49,7 +51,7 @@ func (r *RawRPCResponse) FinishUnmarshal(v interface{}) (interface{}, *btcjson.E
 	}
 	if r.Result != nil {
 		if err := json.Unmarshal([]byte(*r.Result), &v); err != nil {
-			return nil, &btcjson.Error{
+			return nil, btcjson.Error{
 				Code:    btcjson.ErrParse.Code,
 				Message: err.Error(),
 			}

rpcclient.go

@@ -269,20 +269,19 @@ func unmarshalNotification(s string) (btcjson.Cmd, error) {
 
 // GetBestBlock gets both the block height and hash of the best block
 // in the main chain.
-func GetBestBlock(rpc ServerConn) (*btcws.GetBestBlockResult, *btcjson.Error) {
+func GetBestBlock(rpc ServerConn) (*btcws.GetBestBlockResult, error) {
 	cmd := btcws.NewGetBestBlockCmd(<-NewJSONID)
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData btcws.GetBestBlockResult
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return nil, jsonErr
+	if _, err := response.FinishUnmarshal(&resultData); err != nil {
+		return nil, err
 	}
 	return &resultData, nil
 }
 
 // GetBlock requests details about a block with the given hash.
-func GetBlock(rpc ServerConn, blockHash string) (*btcjson.BlockResult, *btcjson.Error) {
+func GetBlock(rpc ServerConn, blockHash string) (*btcjson.BlockResult, error) {
 	// NewGetBlockCmd should never fail with no optargs.  If this does fail,
 	// panic now rather than later.
 	cmd, err := btcjson.NewGetBlockCmd(<-NewJSONID, blockHash)
@@ -292,64 +291,62 @@ func GetBlock(rpc ServerConn, blockHash string) (*btcjson.BlockResult, *btcjson.
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData btcjson.BlockResult
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return nil, jsonErr
+	if _, err := response.FinishUnmarshal(&resultData); err != nil {
+		return nil, err
 	}
 	return &resultData, nil
 }
 
 // GetCurrentNet requests the network a bitcoin RPC server is running on.
-func GetCurrentNet(rpc ServerConn) (btcwire.BitcoinNet, *btcjson.Error) {
+func GetCurrentNet(rpc ServerConn) (btcwire.BitcoinNet, error) {
 	cmd := btcws.NewGetCurrentNetCmd(<-NewJSONID)
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData uint32
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return 0, jsonErr
+	if _, err := response.FinishUnmarshal(&resultData); err != nil {
+		return 0, err
 	}
 	return btcwire.BitcoinNet(resultData), nil
 }
 
 // NotifyBlocks requests blockconnected and blockdisconnected notifications.
-func NotifyBlocks(rpc ServerConn) *btcjson.Error {
+func NotifyBlocks(rpc ServerConn) error {
 	cmd := btcws.NewNotifyBlocksCmd(<-NewJSONID)
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
-	_, jsonErr := response.FinishUnmarshal(nil)
-	return jsonErr
+	_, err := response.FinishUnmarshal(nil)
+	return err
 }
 
 // NotifyReceived requests notifications for new transactions that spend
 // to any of the addresses in addrs.
-func NotifyReceived(rpc ServerConn, addrs []string) *btcjson.Error {
+func NotifyReceived(rpc ServerConn, addrs []string) error {
 	cmd := btcws.NewNotifyReceivedCmd(<-NewJSONID, addrs)
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
-	_, jsonErr := response.FinishUnmarshal(nil)
-	return jsonErr
+	_, err := response.FinishUnmarshal(nil)
+	return err
 }
 
 // NotifySpent requests notifications for when a transaction is processed which
 // spends op.
-func NotifySpent(rpc ServerConn, outpoints []*btcwire.OutPoint) *btcjson.Error {
+func NotifySpent(rpc ServerConn, outpoints []*btcwire.OutPoint) error {
 	ops := make([]btcws.OutPoint, 0, len(outpoints))
 	for _, op := range outpoints {
 		ops = append(ops, *btcws.NewOutPointFromWire(op))
 	}
 	cmd := btcws.NewNotifySpentCmd(<-NewJSONID, ops)
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
-	_, jsonErr := response.FinishUnmarshal(nil)
-	return jsonErr
+	_, err := response.FinishUnmarshal(nil)
+	return err
 }
 
 // Rescan requests a blockchain rescan for transactions to any number of
 // addresses and notifications to inform wallet about such transactions.
 func Rescan(rpc ServerConn, beginBlock int32, addrs []string,
-	outpoints []*btcwire.OutPoint) *btcjson.Error {
+	outpoints []*btcwire.OutPoint) error {
 
-	ops := make([]btcws.OutPoint, len(outpoints))
-	for i := range outpoints {
-		ops[i] = *btcws.NewOutPointFromWire(outpoints[i])
+	ops := make([]btcws.OutPoint, 0, len(outpoints))
+	for _, op := range outpoints {
+		ops = append(ops, *btcws.NewOutPointFromWire(op))
 	}
 	// NewRescanCmd should never fail with no optargs.  If this does fail,
 	// panic now rather than later.
@@ -358,12 +355,12 @@ func Rescan(rpc ServerConn, beginBlock int32, addrs []string,
 		panic(err)
 	}
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
-	_, jsonErr := response.FinishUnmarshal(nil)
-	return jsonErr
+	_, err = response.FinishUnmarshal(nil)
+	return err
 }
 
 // SendRawTransaction sends a hex-encoded transaction for relay.
-func SendRawTransaction(rpc ServerConn, hextx string) (txid string, error *btcjson.Error) {
+func SendRawTransaction(rpc ServerConn, hextx string) (txid string, err error) {
 	// NewSendRawTransactionCmd should never fail.  In the exceptional case
 	// where it does, panic here rather than later.
 	cmd, err := btcjson.NewSendRawTransactionCmd(<-NewJSONID, hextx)
@@ -373,11 +370,8 @@ func SendRawTransaction(rpc ServerConn, hextx string) (txid string, error *btcjs
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData string
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return "", jsonErr
-	}
-	return resultData, nil
+	_, err = response.FinishUnmarshal(&resultData)
+	return resultData, err
 }
 
 // GetRawTransaction returns a future representing a pending GetRawTransaction
@@ -396,22 +390,21 @@ func GetRawTransactionAsync(rpc ServerConn, txsha *btcwire.ShaHash) chan RawRPCR
 // GetRawTransactionAsyncResult waits for the pending command in request -
 // the reqsult of a previous GetRawTransactionAsync() call - and returns either
 // the requested transaction, or an error.
-func GetRawTransactionAsyncResult(request chan RawRPCResponse) (*btcutil.Tx,
-	*btcjson.Error) {
+func GetRawTransactionAsyncResult(request chan RawRPCResponse) (*btcutil.Tx, error) {
 	response := <-request
 
 	var resultData string
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return nil, jsonErr
+	_, err := response.FinishUnmarshal(&resultData)
+	if err != nil {
+		return nil, err
 	}
 	serializedTx, err := hex.DecodeString(resultData)
 	if err != nil {
-		return nil, &btcjson.ErrDecodeHexString
+		return nil, btcjson.ErrDecodeHexString
 	}
 	utx, err := btcutil.NewTxFromBytes(serializedTx)
 	if err != nil {
-		return nil, &btcjson.ErrDeserialization
+		return nil, btcjson.ErrDeserialization
 	}
 	return utx, nil
 }
@@ -419,26 +412,7 @@ func GetRawTransactionAsyncResult(request chan RawRPCResponse) (*btcutil.Tx,
 // GetRawTransaction sends the non-verbose version of a getrawtransaction
 // request to receive the serialized transaction referenced by txsha.  If
 // successful, the transaction is decoded and returned as a btcutil.Tx.
-func GetRawTransaction(rpc ServerConn, txsha *btcwire.ShaHash) (*btcutil.Tx, *btcjson.Error) {
+func GetRawTransaction(rpc ServerConn, txsha *btcwire.ShaHash) (*btcutil.Tx, error) {
 	resp := GetRawTransactionAsync(rpc, txsha)
 	return GetRawTransactionAsyncResult(resp)
 }
-
-// VerboseGetRawTransaction sends the verbose version of a getrawtransaction
-// request to receive details about a transaction.
-func VerboseGetRawTransaction(rpc ServerConn, txsha *btcwire.ShaHash) (*btcjson.TxRawResult, *btcjson.Error) {
-	// NewGetRawTransactionCmd should never fail with a single optarg.  If
-	// it does, panic now rather than later.
-	cmd, err := btcjson.NewGetRawTransactionCmd(<-NewJSONID, txsha.String(), 1)
-	if err != nil {
-		panic(err)
-	}
-	response := <-rpc.SendRequest(NewServerRequest(cmd))
-
-	var resultData btcjson.TxRawResult
-	_, jsonErr := response.FinishUnmarshal(&resultData)
-	if jsonErr != nil {
-		return nil, jsonErr
-	}
-	return &resultData, nil
-}

sockets.go

@@ -212,23 +212,13 @@ func genCertPair(certFile, keyFile string) error {
 	return nil
 }
 
-// ParseRequest parses a command or notification out of a JSON-RPC request,
-// returning any errors as a JSON-RPC error.
-func ParseRequest(msg []byte) (btcjson.Cmd, *btcjson.Error) {
-	cmd, err := btcjson.ParseMarshaledCmd(msg)
-	if err != nil || cmd.Id() == nil {
-		return cmd, &btcjson.ErrInvalidRequest
-	}
-	return cmd, nil
-}
-
 // ReplyToFrontend responds to a marshaled JSON-RPC request with a
 // marshaled JSON-RPC response for both standard and extension
 // (websocket) clients.  The returned error is ErrBadAuth if a
 // missing, incorrect, or duplicate authentication request is
 // received.
 func (s *server) ReplyToFrontend(msg []byte, ws, authenticated bool) ([]byte, error) {
-	cmd, jsonErr := ParseRequest(msg)
+	cmd, parseErr := btcjson.ParseMarshaledCmd(msg)
 	var id interface{}
 	if cmd != nil {
 		id = cmd.Id()
@@ -259,10 +249,10 @@ func (s *server) ReplyToFrontend(msg []byte, ws, authenticated bool) ([]byte, er
 		return nil, nil
 	}
 
-	if jsonErr != nil {
+	if parseErr != nil {
 		response := btcjson.Reply{
 			Id:    &id,
-			Error: jsonErr,
+			Error: &btcjson.ErrInvalidRequest,
 		}
 		mresponse, err := json.Marshal(response)
 		// We expect the marshal to succeed.  If it doesn't, it
@@ -725,16 +715,18 @@ func BtcdConnect(certificates []byte) (*BtcdRPCConn, error) {
 // single TrackSince function (or similar) which requests address
 // notifications and performs the rescan since some block height.
 func Handshake(rpc ServerConn) error {
-	net, jsonErr := GetCurrentNet(rpc)
-	if jsonErr != nil {
-		return jsonErr
+	net, err := GetCurrentNet(rpc)
+	if err != nil {
+		return err
 	}
 	if net != activeNet.Net {
 		return errors.New("btcd and btcwallet running on different Bitcoin networks")
 	}
 
 	// Request notifications for connected and disconnected blocks.
-	NotifyBlocks(rpc)
+	if err := NotifyBlocks(rpc); err != nil {
+		return err
+	}
 
 	// Get current best block.  If this is before than the oldest
 	// saved block hash, assume that this btcd instance is not yet
@@ -767,8 +759,7 @@ func Handshake(rpc ServerConn) error {
 		log.Debugf("Checking for previous saved block with height %v hash %v",
 			bs.Height, bs.Hash)
 
-		_, jsonErr := GetBlock(rpc, bs.Hash.String())
-		if jsonErr != nil {
+		if _, err := GetBlock(rpc, bs.Hash.String()); err != nil {
 			continue
 		}