From 1f32fe05b0ee7c0cdf697deca69e6389c3ed425e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= Date: Thu, 27 Aug 2020 15:17:07 +0200 Subject: [PATCH] Make not found error more verbose (add name) --- api.go | 28 ++++++++++++------------- api_test.go | 4 ++-- cluster.go | 2 +- executor.go | 46 +++++++++++++++++++++--------------------- holder.go | 6 +++--- http/client.go | 2 +- index.go | 2 +- pilosa.go | 8 +++----- server/handler_test.go | 8 ++++---- test/holder.go | 3 ++- 10 files changed, 54 insertions(+), 55 deletions(-) diff --git a/api.go b/api.go index 875d350d6..ef535b879 100644 --- a/api.go +++ b/api.go @@ -196,7 +196,7 @@ func (api *API) Index(ctx context.Context, indexName string) (*Index, error) { index := api.holder.Index(indexName) if index == nil { - return nil, newNotFoundError(ErrIndexNotFound) + return nil, newNotFoundError(ErrIndexNotFound, indexName) } return index, nil } @@ -252,7 +252,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str // Find index. index := api.holder.Index(indexName) if index == nil { - return nil, newNotFoundError(ErrIndexNotFound) + return nil, newNotFoundError(ErrIndexNotFound, indexName) } // Create field. @@ -287,7 +287,7 @@ func (api *API) Field(ctx context.Context, indexName, fieldName string) (*Field, field := api.holder.Field(indexName, fieldName) if field == nil { - return nil, newNotFoundError(ErrFieldNotFound) + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } return field, nil } @@ -378,7 +378,7 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, field := api.holder.Field(indexName, fieldName) if field == nil { - return newNotFoundError(ErrFieldNotFound) + return newNotFoundError(ErrFieldNotFound, fieldName) } // only set and time fields are supported @@ -441,7 +441,7 @@ func (api *API) DeleteField(ctx context.Context, indexName string, fieldName str // Find index. index := api.holder.Index(indexName) if index == nil { - return newNotFoundError(ErrIndexNotFound) + return newNotFoundError(ErrIndexNotFound, indexName) } // Delete field from the index. @@ -472,7 +472,7 @@ func (api *API) DeleteAvailableShard(_ context.Context, indexName, fieldName str // Find field. field := api.holder.Field(indexName, fieldName) if field == nil { - return newNotFoundError(ErrFieldNotFound) + return newNotFoundError(ErrFieldNotFound, fieldName) } // Delete shard from the cache. @@ -514,13 +514,13 @@ func (api *API) ExportCSV(ctx context.Context, indexName string, fieldName strin // Find index. index := api.holder.Index(indexName) if index == nil { - return newNotFoundError(ErrIndexNotFound) + return newNotFoundError(ErrIndexNotFound, indexName) } // Find field from the index. field := index.Field(fieldName) if field == nil { - return newNotFoundError(ErrFieldNotFound) + return newNotFoundError(ErrFieldNotFound, fieldName) } // Find the fragment. @@ -768,7 +768,7 @@ func (api *API) Views(ctx context.Context, indexName string, fieldName string) ( // Retrieve views. f := api.holder.Field(indexName, fieldName) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } // Fetch views. @@ -788,7 +788,7 @@ func (api *API) DeleteView(ctx context.Context, indexName string, fieldName stri // Retrieve field. f := api.holder.Field(indexName, fieldName) if f == nil { - return ErrFieldNotFound + return newNotFoundError(ErrFieldNotFound, fieldName) } // Delete the view. @@ -825,7 +825,7 @@ func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []At // Retrieve index from holder. index := api.holder.Index(indexName) if index == nil { - return nil, newNotFoundError(ErrIndexNotFound) + return nil, newNotFoundError(ErrIndexNotFound, indexName) } // Retrieve local blocks. @@ -863,7 +863,7 @@ func (api *API) FieldAttrDiff(ctx context.Context, indexName string, fieldName s // Retrieve index from holder. f := api.holder.Field(indexName, fieldName) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } // Retrieve local blocks. @@ -1177,14 +1177,14 @@ func (api *API) indexField(indexName string, fieldName string, shard uint64) (*I index := api.holder.Index(indexName) if index == nil { api.server.logger.Printf("fragment error: index=%s, field=%s, shard=%d, err=%s", indexName, fieldName, shard, ErrIndexNotFound.Error()) - return nil, nil, newNotFoundError(ErrIndexNotFound) + return nil, nil, newNotFoundError(ErrIndexNotFound, indexName) } // Retrieve field. field := index.Field(fieldName) if field == nil { api.server.logger.Printf("field error: index=%s, field=%s, shard=%d, err=%s", indexName, fieldName, shard, ErrFieldNotFound.Error()) - return nil, nil, ErrFieldNotFound + return nil, nil, newNotFoundError(ErrFieldNotFound, fieldName) } return index, field, nil } diff --git a/api_test.go b/api_test.go index c019e8f6f..85cb92601 100644 --- a/api_test.go +++ b/api_test.go @@ -135,11 +135,11 @@ func TestAPI_Import(t *testing.T) { index := "rkci" field := "f" - _, err := m0.API.CreateIndex(ctx, index, pilosa.IndexOptions{Keys: false}) + _, err := m1.API.CreateIndex(ctx, index, pilosa.IndexOptions{Keys: false}) if err != nil { t.Fatalf("creating index: %v", err) } - _, err = m0.API.CreateField(ctx, index, field, pilosa.OptFieldTypeSet(pilosa.DefaultCacheType, 100), pilosa.OptFieldKeys()) + _, err = m1.API.CreateField(ctx, index, field, pilosa.OptFieldTypeSet(pilosa.DefaultCacheType, 100), pilosa.OptFieldKeys()) if err != nil { t.Fatalf("creating field: %v", err) } diff --git a/cluster.go b/cluster.go index 272c3aaac..f033ded89 100644 --- a/cluster.go +++ b/cluster.go @@ -1356,7 +1356,7 @@ func (c *cluster) followResizeInstruction(instr *ResizeInstruction) error { // Retrieve field. f := c.holder.Field(src.Index, src.Field) if f == nil { - return ErrFieldNotFound + return newNotFoundError(ErrFieldNotFound, src.Field) } // Create view. diff --git a/executor.go b/executor.go index 1e030b828..2de696f6a 100644 --- a/executor.go +++ b/executor.go @@ -128,7 +128,7 @@ func (e *executor) Execute(ctx context.Context, index string, q *pql.Query, shar idx := e.Holder.Index(index) if idx == nil { - return resp, ErrIndexNotFound + return resp, newNotFoundError(ErrIndexNotFound, index) } // Verify that the number of writes do not exceed the maximum. @@ -241,7 +241,7 @@ func (e *executor) execute(ctx context.Context, index string, q *pql.Query, shar // Round up the number of shards. idx := e.Holder.Index(index) if idx == nil { - return nil, ErrIndexNotFound + return nil, newNotFoundError(ErrIndexNotFound, index) } shards = idx.AvailableShards().Slice() if len(shards) == 0 { @@ -1320,12 +1320,12 @@ func (e *executor) executeRowsShard(_ context.Context, index string, fieldName s // Fetch index. idx := e.Holder.Index(index) if idx == nil { - return nil, ErrIndexNotFound + return nil, newNotFoundError(ErrIndexNotFound, index) } // Fetch field. f := e.Holder.Field(index, fieldName) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } // rowIDs is the result set. @@ -1450,7 +1450,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal // Fetch column label from index. idx := e.Holder.Index(index) if idx == nil { - return nil, ErrIndexNotFound + return nil, newNotFoundError(ErrIndexNotFound, index) } // Fetch field name from argument. @@ -1460,7 +1460,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal } f := e.Holder.Field(index, fieldName) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } rowID, rowOK, rowErr := c.UintArg(fieldName) @@ -1554,7 +1554,7 @@ func (e *executor) executeRowBSIGroupShard(ctx context.Context, index string, c f := e.Holder.Field(index, fieldName) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } // EQ null (not implemented: flip frag.NotNull with max ColumnID) @@ -1744,7 +1744,7 @@ func (e *executor) executeNotShard(ctx context.Context, index string, c *pql.Cal // Make sure the index supports existence tracking. idx := e.Holder.Index(index) if idx == nil { - return nil, ErrIndexNotFound + return nil, newNotFoundError(ErrIndexNotFound, index) } else if idx.existenceField() == nil { return nil, errors.Errorf("index does not support existence tracking: %s", index) } @@ -1834,11 +1834,11 @@ func (e *executor) executeClearBit(ctx context.Context, index string, c *pql.Cal // Retrieve field. idx := e.Holder.Index(index) if idx == nil { - return false, ErrIndexNotFound + return false, newNotFoundError(ErrIndexNotFound, index) } f := idx.Field(fieldName) if f == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } // Read fields using labels. @@ -1904,7 +1904,7 @@ func (e *executor) executeClearRow(ctx context.Context, index string, c *pql.Cal } field := e.Holder.Field(index, fieldName) if field == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } switch field.Type() { @@ -1955,7 +1955,7 @@ func (e *executor) executeClearRowShard(ctx context.Context, index string, c *pq field := e.Holder.Field(index, fieldName) if field == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } // Remove the row from all views. @@ -1984,7 +1984,7 @@ func (e *executor) executeSetRow(ctx context.Context, index string, c *pql.Call, } field := e.Holder.Field(index, fieldName) if field == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } if field.Type() != FieldTypeSet { return false, fmt.Errorf("can't Store() on a %s field", field.Type()) @@ -2025,7 +2025,7 @@ func (e *executor) executeSetRowShard(ctx context.Context, index string, c *pql. field := e.Holder.Field(index, fieldName) if field == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } // Retrieve source row. @@ -2085,11 +2085,11 @@ func (e *executor) executeSet(ctx context.Context, index string, c *pql.Call, op // Retrieve field. idx := e.Holder.Index(index) if idx == nil { - return false, ErrIndexNotFound + return false, newNotFoundError(ErrIndexNotFound, index) } f := idx.Field(fieldName) if f == nil { - return false, ErrFieldNotFound + return false, newNotFoundError(ErrFieldNotFound, fieldName) } // Set column on existence field. @@ -2216,7 +2216,7 @@ func (e *executor) executeSetRowAttrs(ctx context.Context, index string, c *pql. // Retrieve field. field := e.Holder.Field(index, fieldName) if field == nil { - return ErrFieldNotFound + return newNotFoundError(ErrFieldNotFound, fieldName) } // Parse labels. @@ -2285,7 +2285,7 @@ func (e *executor) executeBulkSetRowAttrs(ctx context.Context, index string, cal // Retrieve field. f := e.Holder.Field(index, field) if f == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, field) } rowID, ok, err := c.UintArg("_" + rowLabel) @@ -2323,7 +2323,7 @@ func (e *executor) executeBulkSetRowAttrs(ctx context.Context, index string, cal // Retrieve field. field := e.Holder.Field(index, name) if field == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, name) } // Set attributes. @@ -2367,7 +2367,7 @@ func (e *executor) executeSetColumnAttrs(ctx context.Context, index string, c *p // Retrieve index. idx := e.Holder.Index(index) if idx == nil { - return ErrIndexNotFound + return newNotFoundError(ErrIndexNotFound, index) } col, okCol, errCol := c.UintArg("_" + columnLabel) @@ -2857,7 +2857,7 @@ func (e *executor) translateResult(index string, idx *Index, call *pql.Call, res // TODO: It may be useful to cache this field lookup. field := idx.Field(g.Field) if field == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, g.Field) } if field.keys() { key, err := field.translateStore.TranslateID(g.RowID) @@ -2884,7 +2884,7 @@ func (e *executor) translateResult(index string, idx *Index, call *pql.Call, res } if field := idx.Field(fieldName); field == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } else if field.keys() { other.Keys = make([]string, len(result)) for i, id := range result { @@ -3099,7 +3099,7 @@ func newGroupByIterator(rowIDs []RowIDs, children []*pql.Call, filter *Row, inde return nil, errors.Errorf("%s call must have field with valid (string) field name. Got %v of type %[2]T", call.Name, call.Args["_field"]) } if holder.Field(index, fieldName) == nil { - return nil, ErrFieldNotFound + return nil, newNotFoundError(ErrFieldNotFound, fieldName) } gbi.fields[i].Field = fieldName // Fetch fragment. diff --git a/holder.go b/holder.go index a73550c21..de015099e 100644 --- a/holder.go +++ b/holder.go @@ -455,7 +455,7 @@ func (h *Holder) DeleteIndex(name string) error { // Confirm index exists. index := h.index(name) if index == nil { - return newNotFoundError(ErrIndexNotFound) + return newNotFoundError(ErrIndexNotFound, name) } // Close index. @@ -1042,7 +1042,7 @@ func (s *holderSyncer) syncField(index, name string) error { // Retrieve attributes from differing blocks. // Skip update and recomputation if no attributes have changed. m, err := s.Cluster.InternalClient.RowAttrDiff(ctx, &node.URI, index, name, blks) - if err == ErrFieldNotFound { + if errors.Cause(err) == ErrFieldNotFound { continue // field not created remotely yet, skip } else if err != nil { return errors.Wrap(err, "getting differing blocks") @@ -1071,7 +1071,7 @@ func (s *holderSyncer) syncFragment(index, field, view string, shard uint64) err // Retrieve local field. f := s.Holder.Field(index, field) if f == nil { - return ErrFieldNotFound + return newNotFoundError(ErrFieldNotFound, field) } // Ensure view exists locally. diff --git a/http/client.go b/http/client.go index b23678c77..46a6c1a6d 100644 --- a/http/client.go +++ b/http/client.go @@ -999,7 +999,7 @@ func (c *InternalClient) RowAttrDiff(ctx context.Context, uri *pilosa.URI, index resp, err := c.executeRequest(req.WithContext(ctx)) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { - return nil, pilosa.ErrFieldNotFound + return nil, errors.Wrap(pilosa.ErrFieldNotFound, field) } return nil, err } diff --git a/index.go b/index.go index 5c73809c1..95650f0c2 100644 --- a/index.go +++ b/index.go @@ -475,7 +475,7 @@ func (i *Index) DeleteField(name string) error { // Confirm field exists. f := i.field(name) if f == nil { - return newNotFoundError(ErrFieldNotFound) + return newNotFoundError(ErrFieldNotFound, name) } // Close field. diff --git a/pilosa.go b/pilosa.go index 42ab3d3c1..ebabf52df 100644 --- a/pilosa.go +++ b/pilosa.go @@ -108,13 +108,11 @@ func newConflictError(err error) ConflictError { // NotFoundError wraps an error value to signify that a resource was not found // such that in an HTTP scenario, http.StatusNotFound would be returned. -type NotFoundError struct { - error -} +type NotFoundError error // newNotFoundError returns err wrapped in a NotFoundError. -func newNotFoundError(err error) NotFoundError { - return NotFoundError{err} +func newNotFoundError(err error, name string) NotFoundError { + return NotFoundError(errors.WithMessage(err, name)) } // Regular expression to validate index and field names. diff --git a/server/handler_test.go b/server/handler_test.go index c67d98657..1dd3cae64 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -514,7 +514,7 @@ func TestHandler_Endpoints(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/i0/query", strings.NewReader(`Row(row=30)`))) if w.Code != gohttp.StatusBadRequest { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"error":"executing: map reduce: field not found"}`+"\n" { + } else if body := w.Body.String(); body != `{"error":"executing: map reduce: row: field not found"}`+"\n" { t.Fatalf("unexpected body: %q", body) } }) @@ -531,7 +531,7 @@ func TestHandler_Endpoints(t *testing.T) { var resp pilosa.QueryResponse if err := cmd.API.Serializer.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) - } else if s := resp.Err.Error(); s != `executing: map reduce: field not found` { + } else if s := resp.Err.Error(); s != `executing: map reduce: row: field not found` { t.Fatalf("unexpected error: %s", s) } }) @@ -915,7 +915,7 @@ func TestHandler_Endpoints(t *testing.T) { h.ServeHTTP(w, r) if w.Code != gohttp.StatusNotFound { t.Errorf("unexpected status code: %d", w.Code) - } else if w.Body.String() != `{"success":false,"error":{"message":"deleting field: field not found"}}`+"\n" { + } else if w.Body.String() != `{"success":false,"error":{"message":"deleting field: fld1: field not found"}}`+"\n" { t.Errorf("unexpected body: %q", w.Body.String()) } @@ -935,7 +935,7 @@ func TestHandler_Endpoints(t *testing.T) { h.ServeHTTP(w, r) if w.Code != gohttp.StatusNotFound { t.Errorf("unexpected status code: %d", w.Code) - } else if w.Body.String() != `{"success":false,"error":{"message":"deleting index: index not found"}}`+"\n" { + } else if w.Body.String() != `{"success":false,"error":{"message":"deleting index: idx1: index not found"}}`+"\n" { t.Errorf("unexpected body: %q", w.Body.String()) } }) diff --git a/test/holder.go b/test/holder.go index b5e7c1260..c90278bf0 100644 --- a/test/holder.go +++ b/test/holder.go @@ -21,6 +21,7 @@ import ( "github.com/pilosa/pilosa/v2" "github.com/pilosa/pilosa/v2/boltdb" + "github.com/pkg/errors" ) // Holder is a test wrapper for pilosa.Holder. @@ -96,7 +97,7 @@ func (h *Holder) Row(index, field string, rowID uint64) *pilosa.Row { func (h *Holder) ReadRow(index, field string, rowID uint64) *pilosa.Row { f := h.Holder.Field(index, field) if f == nil { - panic(pilosa.ErrFieldNotFound) + panic(errors.WithMessage(pilosa.ErrFieldNotFound, field)) } row, err := f.Row(rowID) if err != nil {