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/35504
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/analysis/app/compare.go b/analysis/app/compare.go
index 7773ed8..073ba5c 100644
--- a/analysis/app/compare.go
+++ b/analysis/app/compare.go
@@ -79,11 +79,7 @@
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 @@
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 @@
Groups: groups,
Labels: labels,
}
- return data, nil
+ return data
}
diff --git a/analysis/appengine/template/compare.html b/analysis/appengine/template/compare.html
index 01a9755..741ca6e 100644
--- a/analysis/appengine/template/compare.html
+++ b/analysis/appengine/template/compare.html
@@ -10,34 +10,38 @@
<input type="submit" value="Search">
</form>
</div>
- <div>
- {{.Benchstat}}
- </div>
- <table>
- <tr>
- <th>label</th>
- {{range $index, $group := .Groups}}
- <th>
- #{{$index}}
- </th>
- {{end}}
- </tr>
- {{range $label, $exists := .Labels}}
- <tr>
- <th>{{$label}}</th>
- {{range $.Groups}}
- <td>
- {{with index .LabelValues $label}}
- [
- {{range $value, $exists := .}}
- {{printf "%q" $value}}
- {{end}}
- ]
+ {{with .Error}}
+ <p>{{.}}</p>
+ {{else}}
+ <div>
+ {{.Benchstat}}
+ </div>
+ <table>
+ <tr>
+ <th>label</th>
+ {{range $index, $group := .Groups}}
+ <th>
+ #{{$index}}
+ </th>
{{end}}
- </td>
+ </tr>
+ {{range $label, $exists := .Labels}}
+ <tr>
+ <th>{{$label}}</th>
+ {{range $.Groups}}
+ <td>
+ {{with index .LabelValues $label}}
+ [
+ {{range $value, $exists := .}}
+ {{printf "%q" $value}}
+ {{end}}
+ ]
+ {{end}}
+ </td>
+ {{end}}
+ </tr>
{{end}}
- </tr>
- {{end}}
- </table>
+ </table>
+ {{end}}
</body>
</html>
diff --git a/storage/client.go b/storage/client.go
index 09f5925..b648d81 100644
--- a/storage/client.go
+++ b/storage/client.go
@@ -6,7 +6,9 @@
package storage
import (
+ "fmt"
"io"
+ "io/ioutil"
"net/http"
"net/url"
@@ -45,6 +47,15 @@
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 @@
// Close frees resources associated with the query.
func (q *Query) Close() error {
- q.body.Close()
+ if q.body != nil {
+ q.body.Close()
+ q.body = nil
+ }
return q.err
}
diff --git a/storage/client_test.go b/storage/client_test.go
index c05ad20..2938430 100644
--- a/storage/client_test.go
+++ b/storage/client_test.go
@@ -17,6 +17,25 @@
"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 {
