Skip to content

Commit

Permalink
Fix tests to expect 200 response code, handle client errors gracefully.
Browse files Browse the repository at this point in the history
  • Loading branch information
@toddboom
toddboom committed Apr 8, 2015
1 parent cfec0ce commit 542178c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 18 deletions.
10 changes: 7 additions & 3 deletions httpd/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,12 +477,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 @@ -508,7 +508,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
30 changes: 15 additions & 15 deletions httpd/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,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 @@ -230,7 +230,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 body != `{"results":[{"error":"database not found"}]}` {
t.Fatalf("unexpected body: %s", body)
Expand Down Expand Up @@ -264,7 +264,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 body != `{"results":[{"error":"database not found"}]}` {
t.Fatalf("unexpected body: %s", body)
Expand Down Expand Up @@ -324,7 +324,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 @@ -342,7 +342,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 @@ -424,7 +424,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 @@ -442,7 +442,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 @@ -476,7 +476,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 body != `{"results":[{"error":"database not found"}]}` {
t.Fatalf("unexpected body: %s", body)
Expand All @@ -494,7 +494,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 @@ -798,7 +798,7 @@ func TestHandler_CreateDataNode_InternalServerError(t *testing.T) {
defer s.Close()

status, body := MustHTTP("POST", s.URL+`/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 @@ -1154,7 +1154,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 @@ -1298,8 +1298,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"},"timestamp": "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 @@ -1317,8 +1317,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 @@ -168,6 +168,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 542178c

Please sign in to comment.