diff --git a/httpd/handler.go b/httpd/handler.go index 60a490af34f..f2cf8a9066d 100644 --- a/httpd/handler.go +++ b/httpd/handler.go @@ -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 } @@ -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) diff --git a/httpd/handler_test.go b/httpd/handler_test.go index d44473e2579..e7a9b36746f 100644 --- a/httpd/handler_test.go +++ b/httpd/handler_test.go @@ -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) @@ -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) @@ -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) @@ -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) } } @@ -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) } } @@ -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) } } @@ -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) } } @@ -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) @@ -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) @@ -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) @@ -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) @@ -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')"}` @@ -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"}` diff --git a/influxdb.go b/influxdb.go index 21136d18aef..fb0dbbfb632 100644 --- a/influxdb.go +++ b/influxdb.go @@ -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.