storage, analysis: report better errors

Show a meaningful error to the user if their query is invalid or
matches zero results.

Change-Id: I8894686b598e008bac418f85e5d6ab229b64ec09
Reviewed-on: https://go-review.googlesource.com/35504Reviewed-by: Russ Cox <rsc@golang.org>

analysis/app/compare.go

@@ -79,11 +79,7 @@ func (a *App) compare(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	data, err := a.compareQuery(q)
-	if err != nil {
-		http.Error(w, err.Error(), 500)
-		return
-	}
+	data := a.compareQuery(q)
 
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	if err := t.Execute(w, data); err != nil {
@@ -94,32 +90,47 @@ func (a *App) compare(w http.ResponseWriter, r *http.Request) {
 
 type compareData struct {
 	Q         string
+	Error     string
 	Benchstat template.HTML
 	Groups    []*resultGroup
 	Labels    map[string]bool
 }
 
-func (a *App) compareQuery(q string) (*compareData, error) {
+func (a *App) compareQuery(q string) *compareData {
 	// Parse query
 	queries := parseQueryString(q)
 
 	// Send requests
 	// TODO(quentin): Issue requests in parallel?
 	var groups []*resultGroup
-	for _, q := range queries {
+	var found int
+	for _, qPart := range queries {
 		group := &resultGroup{}
-		res := a.StorageClient.Query(q)
+		res := a.StorageClient.Query(qPart)
 		defer res.Close() // TODO: Should happen each time through the loop
 		for res.Next() {
 			group.add(res.Result())
+			found++
 		}
-		if err := res.Err(); err != nil {
+		err := res.Err()
+		res.Close()
+		if err != nil {
 			// TODO: If the query is invalid, surface that to the user.
-			return nil, err
+			return &compareData{
+				Q:     q,
+				Error: err.Error(),
+			}
 		}
 		groups = append(groups, group)
 	}
 
+	if found == 0 {
+		return &compareData{
+			Q:     q,
+			Error: "No results matched the query string.",
+		}, nil
+	}
+
 	// Attempt to automatically split results.
 	if len(groups) == 1 {
 		group := groups[0]
@@ -152,5 +163,5 @@ func (a *App) compareQuery(q string) (*compareData, error) {
 		Groups:    groups,
 		Labels:    labels,
 	}
-	return data, nil
+	return data
 }

analysis/appengine/template/compare.html

@@ -10,6 +10,9 @@
         <input type="submit" value="Search">
       </form>
     </div>
+    {{with .Error}}
+    <p>{{.}}</p>
+    {{else}}
       <div>
         {{.Benchstat}}
       </div>
@@ -39,5 +42,6 @@
         </tr>
         {{end}}
       </table>
+    {{end}}
   </body>
 </html>

storage/client.go

@@ -6,7 +6,9 @@
 package storage
 
 import (
+	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/url"
 
@@ -45,6 +47,15 @@ func (c *Client) Query(q string) *Query {
 		return &Query{err: err}
 	}
 
+	if resp.StatusCode != 200 {
+		defer resp.Body.Close()
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			return &Query{err: err}
+		}
+		return &Query{err: fmt.Errorf("%s", body)}
+	}
+
 	br := benchfmt.NewReader(resp.Body)
 
 	return &Query{br: br, body: resp.Body}
@@ -93,7 +104,10 @@ func (q *Query) Err() error {
 
 // Close frees resources associated with the query.
 func (q *Query) Close() error {
+	if q.body != nil {
 		q.body.Close()
+		q.body = nil
+	}
 	return q.err
 }
 

storage/client_test.go

@@ -17,6 +17,25 @@ import (
 	"golang.org/x/perf/storage/benchfmt"
 )
 
+func TestQueryError(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "invalid query", 500)
+	}))
+	defer ts.Close()
+
+	c := &Client{BaseURL: ts.URL}
+
+	q := c.Query("invalid query")
+	defer q.Close()
+
+	if q.Next() {
+		t.Error("Next = true, want false")
+	}
+	if q.Err() == nil {
+		t.Error("Err = nil, want error")
+	}
+}
+
 func TestQuery(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if have, want := r.URL.RequestURI(), "/search?q=key1%3Avalue+key2%3Avalue"; have != want {