Merge pull request #2186 from influxdb/default-status-code-is-now-200

Default status code for queries is now 200
pull/2539/head
Todd Persen 2015-05-11 13:14:54 -07:00
commit 9e839c0771
3 changed files with 39 additions and 42 deletions

View File

@ -243,21 +243,15 @@ func (h *Handler) serveQuery(w http.ResponseWriter, r *http.Request, user *influ
for r := range results {
// write the status header based on the first result returned in the channel
if !statusWritten {
status := http.StatusOK
if r != nil && r.Err != nil {
if isAuthorizationError(r.Err) {
w.WriteHeader(http.StatusUnauthorized)
} else if isMeasurementNotFoundError(r.Err) {
w.WriteHeader(http.StatusOK)
} else if isTagNotFoundError(r.Err) {
w.WriteHeader(http.StatusOK)
} else if isFieldNotFoundError(r.Err) {
w.WriteHeader(http.StatusOK)
} else {
w.WriteHeader(http.StatusInternalServerError)
status = http.StatusUnauthorized
}
} else {
w.WriteHeader(http.StatusOK)
}
w.WriteHeader(status)
statusWritten = true
}
@ -501,12 +495,12 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *influ
w.WriteHeader(http.StatusOK)
return
}
writeError(influxdb.Result{Err: err}, http.StatusInternalServerError)
writeError(influxdb.Result{Err: err}, http.StatusBadRequest)
return
}
if bp.Database == "" {
writeError(influxdb.Result{Err: fmt.Errorf("database is required")}, http.StatusInternalServerError)
writeError(influxdb.Result{Err: fmt.Errorf("database is required")}, http.StatusBadRequest)
return
}
@ -532,7 +526,11 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *influ
}
if index, err := h.server.WriteSeries(bp.Database, bp.RetentionPolicy, points); err != nil {
writeError(influxdb.Result{Err: err}, http.StatusInternalServerError)
if influxdb.IsClientError(err) {
writeError(influxdb.Result{Err: err}, http.StatusBadRequest)
} else {
writeError(influxdb.Result{Err: err}, http.StatusInternalServerError)
}
return
} else {
w.WriteHeader(http.StatusOK)
@ -839,19 +837,6 @@ func isAuthorizationError(err error) bool {
return ok
}
func isMeasurementNotFoundError(err error) bool {
s := err.Error()
return strings.HasPrefix(s, "measurement") && strings.HasSuffix(s, "not found") || strings.Contains(s, "measurement not found")
}
func isTagNotFoundError(err error) bool {
return (strings.HasPrefix(err.Error(), "unknown field or tag name"))
}
func isFieldNotFoundError(err error) bool {
return (strings.HasPrefix(err.Error(), "field not found"))
}
// mapError writes an error result after trying to start a mapper
func mapError(w http.ResponseWriter, err error) {
b, _ := json.Marshal(&influxdb.MapResponse{Err: err.Error()})

View File

@ -224,7 +224,7 @@ func TestHandler_CreateDatabase_Conflict(t *testing.T) {
defer s.Close()
status, body := MustHTTP("GET", s.URL+`/query`, map[string]string{"q": "CREATE DATABASE foo"}, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
} else if body != `{"results":[{"error":"database exists"}]}` {
t.Fatalf("unexpected body: %s", body)
@ -255,7 +255,7 @@ func TestHandler_DropDatabase_NotFound(t *testing.T) {
defer s.Close()
status, body := MustHTTP("GET", s.URL+`/query`, map[string]string{"q": "DROP DATABASE bar"}, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
} else if !matchRegex(`database not found: bar.*`, body) {
t.Fatalf("unexpected body: %s", body)
@ -289,7 +289,7 @@ func TestHandler_RetentionPolicies_DatabaseNotFound(t *testing.T) {
status, body := MustHTTP("GET", s.URL+`/query`, map[string]string{"q": "SHOW RETENTION POLICIES foo"}, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
} else if !matchRegex(`database not found: foo.*`, body) {
t.Fatalf("unexpected body: %s", body)
@ -349,7 +349,7 @@ func TestHandler_CreateRetentionPolicy_DatabaseNotFound(t *testing.T) {
query := map[string]string{"q": "CREATE RETENTION POLICY bar ON foo DURATION 1h REPLICATION 1"}
status, _ := MustHTTP("GET", s.URL+`/query`, query, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
}
}
@ -367,7 +367,7 @@ func TestHandler_CreateRetentionPolicy_Conflict(t *testing.T) {
status, _ := MustHTTP("GET", s.URL+`/query`, query, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
}
}
@ -449,7 +449,7 @@ func TestHandler_UpdateRetentionPolicy_DatabaseNotFound(t *testing.T) {
status, _ := MustHTTP("GET", s.URL+`/query`, query, nil, "")
// Verify response.
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
}
}
@ -467,7 +467,7 @@ func TestHandler_UpdateRetentionPolicy_NotFound(t *testing.T) {
status, _ := MustHTTP("GET", s.URL+`/query`, query, nil, "")
// Verify response.
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
}
}
@ -501,7 +501,7 @@ func TestHandler_DeleteRetentionPolicy_DatabaseNotFound(t *testing.T) {
query := map[string]string{"q": "DROP RETENTION POLICY bar ON qux"}
status, body := MustHTTP("GET", s.URL+`/query`, query, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
} else if !matchRegex(`database not found: .*qux.*`, body) {
t.Fatalf("unexpected body: %s", body)
@ -519,7 +519,7 @@ func TestHandler_DeleteRetentionPolicy_NotFound(t *testing.T) {
query := map[string]string{"q": "DROP RETENTION POLICY bar ON foo"}
status, body := MustHTTP("GET", s.URL+`/query`, query, nil, "")
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d", status)
} else if body != `{"results":[{"error":"retention policy not found"}]}` {
t.Fatalf("unexpected body: %s", body)
@ -821,7 +821,7 @@ func TestHandler_CreateDataNode_InternalServerError(t *testing.T) {
defer s.Close()
status, body := MustHTTP("POST", s.URL+`/data/data_nodes`, nil, nil, `{"url":""}`)
if status != http.StatusInternalServerError {
if status != http.StatusOK {
t.Fatalf("unexpected status: %d, %s", status, body)
} else if body != `data node url required` {
t.Fatalf("unexpected body: %s", body)
@ -1177,7 +1177,7 @@ func TestHandler_serveWriteSeriesWithNoFields(t *testing.T) {
expected := fmt.Sprintf(`{"error":"%s"}`, influxdb.ErrFieldsRequired.Error())
if status != http.StatusInternalServerError {
if status != http.StatusBadRequest {
t.Fatalf("unexpected status: %d", status)
} else if body != expected {
t.Fatalf("result mismatch:\n\texp=%s\n\tgot=%s\n", expected, body)
@ -1337,8 +1337,8 @@ func TestHandler_serveWriteSeries_invalidJSON(t *testing.T) {
status, body := MustHTTP("POST", s.URL+`/write`, nil, nil, `{"database" : foo", "retentionPolicy" : "bar", "points": [{"name": "cpu", "tags": {"host": "server01"},"time": "2009-11-10T23:00:00Z","fields": {"value": 100}}]}`)
if status != http.StatusInternalServerError {
t.Fatalf("unexpected status: expected: %d, actual: %d", http.StatusInternalServerError, status)
if status != http.StatusBadRequest {
t.Fatalf("unexpected status: expected: %d, actual: %d", http.StatusBadRequest, status)
}
response := `{"error":"invalid character 'o' in literal false (expecting 'a')"}`
@ -1356,8 +1356,8 @@ func TestHandler_serveWriteSeries_noDatabaseSpecified(t *testing.T) {
status, body := MustHTTP("POST", s.URL+`/write`, nil, nil, `{}`)
if status != http.StatusInternalServerError {
t.Fatalf("unexpected status: expected: %d, actual: %d", http.StatusInternalServerError, status)
if status != http.StatusBadRequest {
t.Fatalf("unexpected status: expected: %d, actual: %d", http.StatusBadRequest, status)
}
response := `{"error":"database is required"}`

View File

@ -189,6 +189,18 @@ func isAuthorizationError(err error) bool {
return ok
}
// IsClientError indicates whether an error is a known client error.
func IsClientError(err error) bool {
if err == ErrFieldsRequired {
return true
}
if err == ErrFieldTypeConflict {
return true
}
return false
}
// mustMarshal encodes a value to JSON.
// This will panic if an error occurs. This should only be used internally when
// an invalid marshal will cause corruption and a panic is appropriate.