R4R: Tx Query return values

PENDING.md

@@ -3,6 +3,7 @@
 BREAKING CHANGES
 
 * Gaia REST API
+  * [\#3642](https://github.com/cosmos/cosmos-sdk/pull/3642) `GET /tx/{hash}` now returns `404` instead of `500` if the transaction is not found
 
 * Gaia CLI
 

client/lcd/lcd_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"regexp"
+	"strings"
 	"testing"
 	"time"
 
@@ -503,6 +504,17 @@ func TestTxs(t *testing.T) {
 	txs = getTransactions(t, port, fmt.Sprintf("recipient=%s", receiveAddr.String()))
 	require.Len(t, txs, 1)
 	require.Equal(t, resultTx.Height, txs[0].Height)
+
+	// query transaction that doesn't exist
+	validTxHash := "9ADBECAAD8DACBEC3F4F535704E7CF715C765BDCEDBEF086AFEAD31BA664FB0B"
+	res, body := getTransactionRequest(t, port, validTxHash)
+	require.True(t, strings.Contains(body, validTxHash))
+	require.Equal(t, http.StatusNotFound, res.StatusCode)
+
+	// bad query string
+	res, body = getTransactionRequest(t, port, "badtxhash")
+	require.True(t, strings.Contains(body, "encoding/hex"))
+	require.Equal(t, http.StatusInternalServerError, res.StatusCode)
 }
 
 func TestPoolParamsQuery(t *testing.T) {

client/lcd/test_helpers.go

@@ -504,14 +504,18 @@ func getValidatorSets(t *testing.T, port string, height int, expectFail bool) rp
 // GET /txs/{hash} get tx by hash
 func getTransaction(t *testing.T, port string, hash string) sdk.TxResponse {
 	var tx sdk.TxResponse
-	res, body := Request(t, port, "GET", fmt.Sprintf("/txs/%s", hash), nil)
+	res, body := getTransactionRequest(t, port, hash)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 
 	err := cdc.UnmarshalJSON([]byte(body), &tx)
 	require.NoError(t, err)
 	return tx
 }
 
+func getTransactionRequest(t *testing.T, port, hash string) (*http.Response, string) {
+	return Request(t, port, "GET", fmt.Sprintf("/txs/%s", hash), nil)
+}
+
 // POST /txs broadcast txs
 
 // GET /txs search transactions

client/tx/query.go

@@ -4,6 +4,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/mux"
 	"github.com/spf13/cobra"
@@ -26,18 +27,18 @@ func QueryTxCmd(cdc *codec.Codec) *cobra.Command {
 		Short: "Matches this txhash over all committed blocks",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// find the key to look up the account
-			hashHexStr := args[0]
-
 			cliCtx := context.NewCLIContext().WithCodec(cdc)
 
-			output, err := queryTx(cdc, cliCtx, hashHexStr)
+			output, err := queryTx(cdc, cliCtx, args[0])
 			if err != nil {
 				return err
 			}
 
-			fmt.Println(string(output))
-			return nil
+			if output.Empty() {
+				return fmt.Errorf("No transaction found with hash %s", args[0])
+			}
+
+			return cliCtx.PrintOutput(output)
 		},
 	}
 
@@ -48,50 +49,44 @@ func QueryTxCmd(cdc *codec.Codec) *cobra.Command {
 	return cmd
 }
 
-func queryTx(cdc *codec.Codec, cliCtx context.CLIContext, hashHexStr string) ([]byte, error) {
+func queryTx(cdc *codec.Codec, cliCtx context.CLIContext, hashHexStr string) (out sdk.TxResponse, err error) {
 	hash, err := hex.DecodeString(hashHexStr)
 	if err != nil {
-		return nil, err
+		return out, err
 	}
 
 	node, err := cliCtx.GetNode()
 	if err != nil {
-		return nil, err
+		return out, err
 	}
 
 	res, err := node.Tx(hash, !cliCtx.TrustNode)
 	if err != nil {
-		return nil, err
+		return out, err
 	}
 
-	if !cliCtx.TrustNode {
-		err := ValidateTxResult(cliCtx, res)
-		if err != nil {
-			return nil, err
-		}
+	if err = ValidateTxResult(cliCtx, res); !cliCtx.TrustNode && err != nil {
+		return out, err
 	}
 
-	info, err := formatTxResult(cdc, res)
-	if err != nil {
-		return nil, err
+	if out, err = formatTxResult(cdc, res); err != nil {
+		return out, err
 	}
 
-	if cliCtx.Indent {
-		return cdc.MarshalJSONIndent(info, "", "  ")
-	}
-	return cdc.MarshalJSON(info)
+	return out, nil
 }
 
 // ValidateTxResult performs transaction verification
 func ValidateTxResult(cliCtx context.CLIContext, res *ctypes.ResultTx) error {
-	check, err := cliCtx.Verify(res.Height)
-	if err != nil {
-		return err
-	}
-
-	err = res.Proof.Validate(check.Header.DataHash)
-	if err != nil {
-		return err
+	if !cliCtx.TrustNode {
+		check, err := cliCtx.Verify(res.Height)
+		if err != nil {
+			return err
+		}
+		err = res.Proof.Validate(check.Header.DataHash)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -118,17 +113,26 @@ func parseTx(cdc *codec.Codec, txBytes []byte) (sdk.Tx, error) {
 
 // REST
 
-// transaction query REST handler
+// QueryTxRequestHandlerFn transaction query REST handler
 func QueryTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		hashHexStr := vars["hash"]
 
 		output, err := queryTx(cdc, cliCtx, hashHexStr)
 		if err != nil {
+			if strings.Contains(err.Error(), hashHexStr) {
+				rest.WriteErrorResponse(w, http.StatusNotFound, err.Error())
+				return
+			}
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
+
+		if output.Empty() {
+			rest.WriteErrorResponse(w, http.StatusNotFound, fmt.Sprintf("no transaction found with hash %s", hashHexStr))
+		}
+
 		rest.PostProcessResponse(w, cdc, output, cliCtx.Indent)
 	}
 }

types/result.go

@@ -54,6 +54,7 @@ type TxResponse struct {
 	Tx        Tx         `json:"tx,omitempty"`
 }
 
+// NewResponseResultTx returns a TxResponse given a ResultTx from tendermint
 func NewResponseResultTx(res *ctypes.ResultTx, tx Tx) TxResponse {
 	if res == nil {
 		return TxResponse{}
@@ -73,6 +74,7 @@ func NewResponseResultTx(res *ctypes.ResultTx, tx Tx) TxResponse {
 	}
 }
 
+// NewResponseFormatBroadcastTxCommit returns a TxResponse given a ResultBroadcastTxCommit from tendermint
 func NewResponseFormatBroadcastTxCommit(res *ctypes.ResultBroadcastTxCommit) TxResponse {
 	if res == nil {
 		return TxResponse{}
@@ -95,8 +97,10 @@ func NewResponseFormatBroadcastTxCommit(res *ctypes.ResultBroadcastTxCommit) TxR
 		Tags:      TagsToStringTags(res.DeliverTx.Tags),
 		Codespace: res.DeliverTx.Codespace,
 	}
+
 }
 
+// NewResponseFormatBroadcastTx returns a TxResponse given a ResultBroadcastTx from tendermint
 func NewResponseFormatBroadcastTx(res *ctypes.ResultBroadcastTx) TxResponse {
 	if res == nil {
 		return TxResponse{}
@@ -156,3 +160,8 @@ func (r TxResponse) String() string {
 
 	return strings.TrimSpace(sb.String())
 }
+
+// Empty returns true if the response is empty
+func (r TxResponse) Empty() bool {
+	return r.TxHash == "" && r.Log == ""
+}