Skip to content

Commit

Permalink
Fix /api/search endpoint encoding error
Browse files Browse the repository at this point in the history
Previously this endpoint was always returning an empty response for
valid requests. This is because the last line failed to encode the
SearchResult as JSON. After some investigation I can see that this is
because we default the Progress fields to `-Inf`. Since these progress
fields only seemm to be applicable for streaming use cases that use RPC
and don't encode as JSON then it seemed like the best fix was to simply
omit them in the JSON response.

This change also adds error handling to the JSON encoding so that these
errors will be simpler to debug in future. A test is also included that
reproduces the issue.

You can reproduce this issue and the fix locally by just curling the
search endpoint like:

```bash
curl -H 'Accept: application/json' -XPOST -d '{"Q":"a.*"}' 'http://127.0.0.1:6070/api/search' -i
```
  • Loading branch information
DylanGriffith committed Dec 21, 2022
1 parent 0d86a86 commit 1ac49ce
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
5 changes: 4 additions & 1 deletion api.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,10 @@ func (p *Progress) sizeBytes() uint64 {
// SearchResult contains search matches and extra data
type SearchResult struct {
Stats
Progress

// Do not encode this as we cannot encode -Inf in JSON
Progress `json:"-"`

Files []FileMatch

// RepoURLs holds a repo => template string map.
Expand Down
7 changes: 6 additions & 1 deletion json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ func (s *jsonSearcher) jsonSearch(w http.ResponseWriter, req *http.Request) {
return
}

json.NewEncoder(w).Encode(jsonSearchReply{searchResult})
err = json.NewEncoder(w).Encode(jsonSearchReply{searchResult})

if err != nil {
jsonError(w, http.StatusInternalServerError, err.Error())
return
}
}

func jsonError(w http.ResponseWriter, statusCode int, err string) {
Expand Down
33 changes: 33 additions & 0 deletions json/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"io"
"math"
"net/http"
"net/http/httptest"
"reflect"
Expand Down Expand Up @@ -86,6 +87,38 @@ func TestClientServer(t *testing.T) {
}
}

func TestProgressNotEncodedInSearch(t *testing.T) {
searchQuery := "hello"
mock := &mockSearcher.MockSearcher{
WantSearch: mustParse(searchQuery),
SearchResult: &zoekt.SearchResult{
// Validate that Progress is ignored as we cannot encode -Inf
Progress: zoekt.Progress{
Priority: math.Inf(-1),
MaxPendingPriority: math.Inf(-1),
},
Files: []zoekt.FileMatch{},
},
}

ts := httptest.NewServer(zjson.JSONServer(mock))
defer ts.Close()

searchBody, err := json.Marshal(struct{ Q string }{Q: searchQuery})
if err != nil {
t.Fatal(err)
}
r, err := http.Post(ts.URL+"/search", "application/json", bytes.NewBuffer(searchBody))
if err != nil {
t.Fatal(err)
}

if r.StatusCode != 200 {
body, _ := io.ReadAll(r.Body)
t.Fatalf("Got status code %d, err %s", r.StatusCode, string(body))
}
}

func mustParse(s string) query.Q {
q, err := query.Parse(s)
if err != nil {
Expand Down

0 comments on commit 1ac49ce

Please sign in to comment.