Browse Source

fix(discover2) Return better errors for is: and <col>: queries (#16774)

Right now if a user has a space after a filter in their query,
e.g. '<filter>: something', we assume that means search for empty strings in
<filter> and a message search for 'something'. Instead make this an error. If
users want to search for the empty string they can specify that explicity
(<filter>:""). There's only one saved query that has this bad syntax, and it
doesn't return results anyways.

Also, the error message for  when a user tries to the is: filter in discover is
not very friendly. Just want to push people back to the issue page so they don't
get frustrated.
evanh 5 years ago
parent
commit
9683003d6e
2 changed files with 21 additions and 4 deletions
  1. 13 2
      src/sentry/api/event_search.py
  2. 8 2
      tests/sentry/api/test_event_search.py

+ 13 - 2
src/sentry/api/event_search.py

@@ -95,13 +95,15 @@ boolean_term         = (paren_term / search_term) space? (boolean_operator space
 paren_term           = space? open_paren space? (paren_term / boolean_term)+ space? closed_paren space?
 search_term          = key_val_term / quoted_raw_search / raw_search
 key_val_term         = space? (tag_filter / time_filter / rel_time_filter / specific_time_filter
-                       / numeric_filter / aggregate_filter / aggregate_date_filter / has_filter / is_filter / basic_filter)
+                       / numeric_filter / aggregate_filter / aggregate_date_filter / has_filter
+                       / is_filter / quoted_basic_filter / basic_filter)
                        space?
 raw_search           = (!key_val_term ~r"\ *([^\ ^\n ()]+)\ *" )*
 quoted_raw_search    = spaces quoted_value spaces
 
 # standard key:val filter
 basic_filter         = negation? search_key sep search_value
+quoted_basic_filter  = negation? search_key sep quoted_value
 # filter for dates
 time_filter          = search_key sep? operator date_format
 # filter for relative dates
@@ -470,9 +472,18 @@ class SearchVisitor(NodeVisitor):
 
         return node.text == "!"
 
+    def visit_quoted_basic_filter(self, node, children):
+        (negation, search_key, _, search_value) = children
+        operator = "!=" if self.is_negated(negation) else "="
+        search_value = SearchValue(search_value)
+        return self._handle_basic_filter(search_key, operator, search_value)
+
     def visit_basic_filter(self, node, children):
         (negation, search_key, _, search_value) = children
         operator = "!=" if self.is_negated(negation) else "="
+        if not search_value.raw_value:
+            raise InvalidSearchQuery("Empty string after '%s:'" % search_key.name)
+
         return self._handle_basic_filter(search_key, operator, search_value)
 
     def _handle_basic_filter(self, search_key, operator, search_value):
@@ -504,7 +515,7 @@ class SearchVisitor(NodeVisitor):
         return SearchFilter(SearchKey(u"tags[%s]" % (search_key.name)), operator, search_value)
 
     def visit_is_filter(self, node, children):
-        raise InvalidSearchQuery('"is" queries are not supported on this search')
+        raise InvalidSearchQuery('"is:" queries are only supported in issue search.')
 
     def visit_search_key(self, node, children):
         key = children[0]

+ 8 - 2
tests/sentry/api/test_event_search.py

@@ -413,6 +413,10 @@ class ParseSearchQueryTest(unittest.TestCase):
                 key=SearchKey(name="device.family"), operator="=", value=SearchValue(raw_value="")
             )
         ]
+        with self.assertRaises(
+            InvalidSearchQuery, expected_regex="Invalid format for numeric search"
+        ):
+            parse_search_query("device.family:")
 
     def test_custom_tag(self):
         assert parse_search_query("fruit:apple release:1.2.1") == [
@@ -472,7 +476,9 @@ class ParseSearchQueryTest(unittest.TestCase):
         ]
 
     def test_is_query_unsupported(self):
-        with self.assertRaises(InvalidSearchQuery):
+        with self.assertRaises(
+            InvalidSearchQuery, expected_regex="queries are only supported in issue search"
+        ):
             parse_search_query("is:unassigned")
 
     def test_key_remapping(self):
@@ -1054,7 +1060,7 @@ class GetSnubaQueryArgsTest(TestCase):
         assert filter.filter_keys == {}
         assert filter.group_ids == []
 
-        filter = get_filter("environment: ")
+        filter = get_filter('environment:""')
         # The '' environment is Null in snuba
         assert filter.conditions == [[["environment", "IS NULL", None]]]
         assert filter.filter_keys == {}