Skip to content

Commit

Permalink
Merge pull request #2186 from influxdb/default-status-code-is-now-200
Browse files Browse the repository at this point in the history 
Default status code for queries is now 200
  • Loading branch information
@toddboom
toddboom committed May 11, 2015
2 parents 0080915 + ea5a321 commit 9e839c0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 42 deletions.
39 changes: 12 additions & 27 deletions httpd/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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)
Expand Down Expand Up @@ -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()})
Expand Down
30 changes: 15 additions & 15 deletions httpd/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')"}`
Expand All @@ -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"}`
Expand Down
12 changes: 12 additions & 0 deletions influxdb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 9e839c0

Please sign in to comment.