mirror of
https://github.com/featurebasedb/featurebase.git
synced 2026-10-08 11:57:51 +00:00
experiment: handle errors more gracefully, but don't stream CreateKeys
We were trying to write an error to a ResponseWriter After attempting to write to it, and this produces messages about superfluous WriteHeaders, which is correct. This patch changes things so that we report messages more clearly and verbosely if we hit them before writing, and if we try to write and fail, we log the message because that's all we can do. This does change semantics slightly, in that now we're marshalling separately from trying to write the marshalled data. I think this is probably a reasonable call because it lets us get diagnostics about a hypothetical encoding problem, but in practice I don't think there should be any encoding problems. So my guess is the actual error will occur in that last line, and be logged to the server console instead of failing to write over HTTP. Also note that this changes some of the messages to include the underlying error they're complaining about. We also merge the create/find and index/field cases because only a couple of lines of code changed between four largeish functions, and we test some of the failure cases. We don't have test coverage on the "field isn't provided" type things because the mux won't actually route things there without them, so far as I know.
This commit is contained in:
Seebs
parent
73f3c01284
commit
8b20406f45
2 changed files with 167 additions and 159 deletions
214
http/handler.go
214
http/handler.go
|
|
@ -3091,7 +3091,7 @@ func (h *Handler) handlePostTranslateIndexDB(w http.ResponseWriter, r *http.Requ
|
|||
resp.write(w, err)
|
||||
}
|
||||
|
||||
func (h *Handler) handleFindIndexKeys(w http.ResponseWriter, r *http.Request) {
|
||||
func (h *Handler) handleFindOrCreateKeys(w http.ResponseWriter, r *http.Request, requireField bool, create bool) {
|
||||
// Verify input and output types
|
||||
if r.Header.Get("Content-Type") != "application/json" {
|
||||
http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType)
|
||||
|
|
@ -3101,178 +3101,76 @@ func (h *Handler) handleFindIndexKeys(w http.ResponseWriter, r *http.Request) {
|
|||
http.Error(w, "Not acceptable", http.StatusNotAcceptable)
|
||||
return
|
||||
}
|
||||
|
||||
indexName, ok := mux.Vars(r)["index"]
|
||||
if !ok {
|
||||
http.Error(w, "index name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
bd, err := readBody(r)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to read body", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
var indexName, fieldName string
|
||||
var keys []string
|
||||
err = json.Unmarshal(bd, &keys)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to decode request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
err := func() error {
|
||||
var ok bool
|
||||
indexName, ok = mux.Vars(r)["index"]
|
||||
if !ok {
|
||||
return errors.New("index name is required")
|
||||
}
|
||||
|
||||
translations, err := h.api.FindIndexKeys(r.Context(), indexName, keys...)
|
||||
if err != nil {
|
||||
http.Error(w, "translating keys", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if requireField {
|
||||
fieldName, ok = mux.Vars(r)["field"]
|
||||
if !ok {
|
||||
return errors.New("field name is required")
|
||||
}
|
||||
}
|
||||
|
||||
err = json.NewEncoder(w).Encode(translations)
|
||||
bd, err := readBody(r)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to read body: %v", err)
|
||||
}
|
||||
|
||||
err = json.Unmarshal(bd, &keys)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to decode request: %v", err)
|
||||
}
|
||||
return nil
|
||||
}()
|
||||
if err != nil {
|
||||
http.Error(w, "encoding result", http.StatusBadRequest)
|
||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||
}
|
||||
var translations map[string]uint64
|
||||
switch {
|
||||
case requireField && create:
|
||||
translations, err = h.api.CreateFieldKeys(r.Context(), indexName, fieldName, keys...)
|
||||
case requireField && !create:
|
||||
translations, err = h.api.FindFieldKeys(r.Context(), indexName, fieldName, keys...)
|
||||
case !requireField && create:
|
||||
translations, err = h.api.CreateIndexKeys(r.Context(), indexName, keys...)
|
||||
case !requireField && !create:
|
||||
translations, err = h.api.FindIndexKeys(r.Context(), indexName, keys...)
|
||||
|
||||
}
|
||||
if err != nil {
|
||||
http.Error(w, fmt.Sprintf("translating keys: %v", err), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
data, err := json.Marshal(translations)
|
||||
if err != nil {
|
||||
http.Error(w, fmt.Sprintf("encoding response: %v", err), http.StatusInternalServerError)
|
||||
}
|
||||
_, err = w.Write(data)
|
||||
if err != nil {
|
||||
h.logger.Printf("writing CreateFieldKeys response: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func (h *Handler) handleFindIndexKeys(w http.ResponseWriter, r *http.Request) {
|
||||
h.handleFindOrCreateKeys(w, r, false, false)
|
||||
}
|
||||
|
||||
func (h *Handler) handleFindFieldKeys(w http.ResponseWriter, r *http.Request) {
|
||||
// Verify input and output types
|
||||
if r.Header.Get("Content-Type") != "application/json" {
|
||||
http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType)
|
||||
return
|
||||
}
|
||||
if !validHeaderAcceptJSON(r.Header) {
|
||||
http.Error(w, "Not acceptable", http.StatusNotAcceptable)
|
||||
return
|
||||
}
|
||||
|
||||
indexName, ok := mux.Vars(r)["index"]
|
||||
if !ok {
|
||||
http.Error(w, "index name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
fieldName, ok := mux.Vars(r)["field"]
|
||||
if !ok {
|
||||
http.Error(w, "field name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
bd, err := readBody(r)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to read body", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
var keys []string
|
||||
err = json.Unmarshal(bd, &keys)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to decode request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
translations, err := h.api.FindFieldKeys(r.Context(), indexName, fieldName, keys...)
|
||||
if err != nil {
|
||||
http.Error(w, "translating keys", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
err = json.NewEncoder(w).Encode(translations)
|
||||
if err != nil {
|
||||
http.Error(w, "encoding result", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
h.handleFindOrCreateKeys(w, r, true, false)
|
||||
}
|
||||
|
||||
func (h *Handler) handleCreateIndexKeys(w http.ResponseWriter, r *http.Request) {
|
||||
// Verify input and output types
|
||||
if r.Header.Get("Content-Type") != "application/json" {
|
||||
http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType)
|
||||
return
|
||||
}
|
||||
if !validHeaderAcceptJSON(r.Header) {
|
||||
http.Error(w, "Not acceptable", http.StatusNotAcceptable)
|
||||
return
|
||||
}
|
||||
|
||||
indexName, ok := mux.Vars(r)["index"]
|
||||
if !ok {
|
||||
http.Error(w, "index name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
bd, err := readBody(r)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to read body", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
var keys []string
|
||||
err = json.Unmarshal(bd, &keys)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to decode request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
translations, err := h.api.CreateIndexKeys(r.Context(), indexName, keys...)
|
||||
if err != nil {
|
||||
http.Error(w, "translating keys", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
err = json.NewEncoder(w).Encode(translations)
|
||||
if err != nil {
|
||||
http.Error(w, "encoding result", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
h.handleFindOrCreateKeys(w, r, false, true)
|
||||
}
|
||||
|
||||
func (h *Handler) handleCreateFieldKeys(w http.ResponseWriter, r *http.Request) {
|
||||
// Verify input and output types
|
||||
if r.Header.Get("Content-Type") != "application/json" {
|
||||
http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType)
|
||||
return
|
||||
}
|
||||
if !validHeaderAcceptJSON(r.Header) {
|
||||
http.Error(w, "Not acceptable", http.StatusNotAcceptable)
|
||||
return
|
||||
}
|
||||
|
||||
indexName, ok := mux.Vars(r)["index"]
|
||||
if !ok {
|
||||
http.Error(w, "index name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
fieldName, ok := mux.Vars(r)["field"]
|
||||
if !ok {
|
||||
http.Error(w, "field name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
bd, err := readBody(r)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to read body", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
var keys []string
|
||||
err = json.Unmarshal(bd, &keys)
|
||||
if err != nil {
|
||||
http.Error(w, "failed to decode request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
translations, err := h.api.CreateFieldKeys(r.Context(), indexName, fieldName, keys...)
|
||||
if err != nil {
|
||||
http.Error(w, "translating keys", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
err = json.NewEncoder(w).Encode(translations)
|
||||
if err != nil {
|
||||
http.Error(w, "encoding result", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
h.handleFindOrCreateKeys(w, r, true, true)
|
||||
}
|
||||
|
||||
func (h *Handler) handleMatchField(w http.ResponseWriter, r *http.Request) {
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import (
|
|||
"fmt"
|
||||
"net"
|
||||
gohttp "net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
pilosa "github.com/molecula/featurebase/v2"
|
||||
|
|
@ -167,6 +168,115 @@ func TestIngestSchemaHandler(t *testing.T) {
|
|||
schemaURL := fmt.Sprintf("%s/internal/schema", m.URL())
|
||||
resp := test.Do(t, "POST", schemaURL, string(schema))
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
t.Errorf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
t.Errorf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
// now, try again, expecting a failure:
|
||||
resp = test.Do(t, "POST", schemaURL, string(schema))
|
||||
if resp.StatusCode != gohttp.StatusConflict {
|
||||
t.Errorf("invalid status: expected 409, got %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTranslationHandlers(t *testing.T) {
|
||||
// reusable data for the tests
|
||||
nameBytes, err := json.Marshal([]string{"a", "b", "c"})
|
||||
if err != nil {
|
||||
t.Fatalf("marshalling json: %v", err)
|
||||
}
|
||||
names := string(nameBytes)
|
||||
|
||||
c := test.MustRunCluster(t, 1)
|
||||
defer c.Close()
|
||||
|
||||
schema := `
|
||||
{
|
||||
"index-name": "example",
|
||||
"primary-key-type": "string",
|
||||
"index-action": "create",
|
||||
"fields": [
|
||||
{
|
||||
"field-name": "stringset",
|
||||
"field-type": "string",
|
||||
"field-options": {
|
||||
"cache-type": "ranked",
|
||||
"cache-size": 100000
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
`
|
||||
m := c.GetPrimary()
|
||||
schemaURL := fmt.Sprintf("%s/internal/schema", m.URL())
|
||||
resp := test.Do(t, "POST", schemaURL, string(schema))
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
t.Errorf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
baseURLs := []string{
|
||||
fmt.Sprintf("%s/internal/translate/index/example/", m.URL()),
|
||||
fmt.Sprintf("%s/internal/translate/field/example/stringset/", m.URL()),
|
||||
fmt.Sprintf("%s/internal/translate/field/example/nonexistent/", m.URL()),
|
||||
}
|
||||
for _, url := range baseURLs {
|
||||
expectFailure := strings.HasSuffix(url, "/nonexistent/")
|
||||
createURL := url + "keys/create"
|
||||
findURL := url + "keys/find"
|
||||
var results map[string]uint64
|
||||
|
||||
if expectFailure {
|
||||
resp := test.Do(t, "POST", findURL, names)
|
||||
if resp.StatusCode != gohttp.StatusInternalServerError {
|
||||
t.Fatalf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
resp = test.Do(t, "POST", createURL, names)
|
||||
if resp.StatusCode != gohttp.StatusInternalServerError {
|
||||
t.Fatalf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// try to find them when they don't exist
|
||||
resp := test.Do(t, "POST", findURL, names)
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
t.Fatalf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
err := json.Unmarshal([]byte(resp.Body), &results)
|
||||
if err != nil {
|
||||
t.Fatalf("unmarshalling result: %v", err)
|
||||
}
|
||||
if len(results) != 0 {
|
||||
t.Fatalf("finding keys before any were set: expected no results, got %d (%q)", len(results), results)
|
||||
}
|
||||
|
||||
// try to create them, but malformed, so we expect an error
|
||||
resp = test.Do(t, "POST", createURL, names[:6])
|
||||
if resp.StatusCode != gohttp.StatusBadRequest {
|
||||
t.Fatalf("invalid status: expected 400, got %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
|
||||
// try to create them
|
||||
resp = test.Do(t, "POST", createURL, names)
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
t.Fatalf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
err = json.Unmarshal([]byte(resp.Body), &results)
|
||||
if err != nil {
|
||||
t.Fatalf("unmarshalling result: %v", err)
|
||||
}
|
||||
if len(results) != 3 {
|
||||
t.Fatalf("finding keys before any were set: expected 3 results, got %d (%q)", len(results), results)
|
||||
}
|
||||
|
||||
// try to find them now that they exist
|
||||
resp = test.Do(t, "POST", findURL, names)
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
t.Fatalf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
}
|
||||
err = json.Unmarshal([]byte(resp.Body), &results)
|
||||
if err != nil {
|
||||
t.Fatalf("unmarshalling result: %v", err)
|
||||
}
|
||||
if len(results) != 3 {
|
||||
t.Fatalf("finding keys before any were set: expected 3 results, got %d (%q)", len(results), results)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue