Browse Source

feat(discover): improve invalid filter message (#24484)

There are a number of users at least on SaaS who are having difficulty
formatting their date-related filters. We should improve our docs and frontend
handling here, but I think this is a reasonable and very easy solution to
reduce the issues. I don't think we have room for a full exposition here, but a
brief description of the expected format seems reasonable.
Alex Xu @ Sentry 4 years ago
parent
commit
008801332a

+ 9 - 3
src/sentry/api/event_search.py

@@ -635,11 +635,17 @@ class SearchVisitor(NodeVisitor):
         # If a date or numeric key gets down to the basic filter, then it means
         # If a date or numeric key gets down to the basic filter, then it means
         # that the value wasn't in a valid format, so raise here.
         # that the value wasn't in a valid format, so raise here.
         if search_key.name in self.date_keys:
         if search_key.name in self.date_keys:
-            raise InvalidSearchQuery(f"{search_key.name}: Invalid format for date field")
+            raise InvalidSearchQuery(
+                f"{search_key.name}: Invalid date: {search_value.raw_value}. Expected +/-duration (e.g. +1h) or ISO 8601-like (e.g. {datetime.now().isoformat()[:-4]})."
+            )
         if search_key.name in self.boolean_keys:
         if search_key.name in self.boolean_keys:
-            raise InvalidSearchQuery(f"{search_key.name}: Invalid format for boolean field")
+            raise InvalidSearchQuery(
+                f"{search_key.name}: Invalid boolean: {search_value.raw_value}. Expected true, 1, false, or 0."
+            )
         if self.is_numeric_key(search_key.name):
         if self.is_numeric_key(search_key.name):
-            raise InvalidSearchQuery(f"{search_key.name}: Invalid format for numeric field")
+            raise InvalidSearchQuery(
+                f"{search_key.name}: Invalid number: {search_value.raw_value}. Expected number then optional k, m, or b suffix (e.g. 500k)."
+            )
 
 
         return SearchFilter(search_key, operator, search_value)
         return SearchFilter(search_key, operator, search_value)
 
 

+ 1 - 1
src/sentry/api/helpers/group_index.py

@@ -98,7 +98,7 @@ def build_query_params_from_request(request, organization, projects, environment
                 parse_search_query(query), projects, request.user, environments
                 parse_search_query(query), projects, request.user, environments
             )
             )
         except InvalidSearchQuery as e:
         except InvalidSearchQuery as e:
-            raise ValidationError(f"Your search query could not be parsed: {e}")
+            raise ValidationError(f"Error parsing search query: {e}")
 
 
         validate_search_filter_permissions(organization, search_filters, request.user)
         validate_search_filter_permissions(organization, search_filters, request.user)
         query_kwargs["search_filters"] = search_filters
         query_kwargs["search_filters"] = search_filters

+ 1 - 1
src/sentry/search/utils.py

@@ -76,7 +76,7 @@ def parse_numeric_value(value, suffix=None):
     try:
     try:
         value = float(value)
         value = float(value)
     except ValueError:
     except ValueError:
-        raise InvalidQuery("Invalid format for numeric field")
+        raise InvalidQuery("Invalid number")
 
 
     if not suffix:
     if not suffix:
         return value
         return value

+ 3 - 3
tests/sentry/api/test_event_search.py

@@ -362,7 +362,7 @@ class ParseSearchQueryTest(unittest.TestCase):
     def test_invalid_date_formats(self):
     def test_invalid_date_formats(self):
         invalid_queries = ["first_seen:hello", "first_seen:123", "first_seen:2018-01-01T00:01ZZ"]
         invalid_queries = ["first_seen:hello", "first_seen:123", "first_seen:2018-01-01T00:01ZZ"]
         for invalid_query in invalid_queries:
         for invalid_query in invalid_queries:
-            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid format for date field"):
+            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid date"):
                 parse_search_query(invalid_query)
                 parse_search_query(invalid_query)
 
 
     def test_specific_time_filter(self):
     def test_specific_time_filter(self):
@@ -689,7 +689,7 @@ class ParseSearchQueryTest(unittest.TestCase):
     def test_invalid_boolean_filter(self):
     def test_invalid_boolean_filter(self):
         invalid_queries = ["stack.in_app:lol", "stack.in_app:123", "stack.in_app:>true"]
         invalid_queries = ["stack.in_app:lol", "stack.in_app:123", "stack.in_app:>true"]
         for invalid_query in invalid_queries:
         for invalid_query in invalid_queries:
-            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid format for boolean field"):
+            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid boolean"):
                 parse_search_query(invalid_query)
                 parse_search_query(invalid_query)
 
 
     def test_numeric_filter(self):
     def test_numeric_filter(self):
@@ -738,7 +738,7 @@ class ParseSearchQueryTest(unittest.TestCase):
     def test_invalid_numeric_fields(self):
     def test_invalid_numeric_fields(self):
         invalid_queries = ["project.id:one", "issue.id:two", "transaction.duration:>hotdog"]
         invalid_queries = ["project.id:one", "issue.id:two", "transaction.duration:>hotdog"]
         for invalid_query in invalid_queries:
         for invalid_query in invalid_queries:
-            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid format for numeric field"):
+            with self.assertRaisesRegexp(InvalidSearchQuery, "Invalid number"):
                 parse_search_query(invalid_query)
                 parse_search_query(invalid_query)
 
 
     def test_invalid_numeric_shorthand(self):
     def test_invalid_numeric_shorthand(self):

+ 1 - 3
tests/sentry/api/test_issue_search.py

@@ -118,9 +118,7 @@ class ParseSearchQueryTest(TestCase):
             'times_seen:"<10"',
             'times_seen:"<10"',
         ]
         ]
         for invalid_query in invalid_queries:
         for invalid_query in invalid_queries:
-            with self.assertRaises(
-                InvalidSearchQuery, expected_regex="Invalid format for numeric search"
-            ):
+            with self.assertRaises(InvalidSearchQuery, expected_regex="Invalid number"):
                 parse_search_query(invalid_query)
                 parse_search_query(invalid_query)
 
 
     def test_boolean_operators_not_allowed(self):
     def test_boolean_operators_not_allowed(self):

+ 3 - 3
tests/snuba/api/endpoints/test_organization_group_index.py

@@ -312,14 +312,14 @@ class GroupListTest(APITestCase, SnubaTestCase):
         assert response.status_code == 400
         assert response.status_code == 400
         assert (
         assert (
             response.data["detail"]
             response.data["detail"]
-            == 'Your search query could not be parsed: Boolean statements containing "OR" or "AND" are not supported in this search'
+            == 'Error parsing search query: Boolean statements containing "OR" or "AND" are not supported in this search'
         )
         )
 
 
         response = self.get_response(sort_by="date", query="title:hello AND title:goodbye")
         response = self.get_response(sort_by="date", query="title:hello AND title:goodbye")
         assert response.status_code == 400
         assert response.status_code == 400
         assert (
         assert (
             response.data["detail"]
             response.data["detail"]
-            == 'Your search query could not be parsed: Boolean statements containing "OR" or "AND" are not supported in this search'
+            == 'Error parsing search query: Boolean statements containing "OR" or "AND" are not supported in this search'
         )
         )
 
 
     def test_invalid_query(self):
     def test_invalid_query(self):
@@ -329,7 +329,7 @@ class GroupListTest(APITestCase, SnubaTestCase):
 
 
         response = self.get_response(sort_by="date", query="timesSeen:>1t")
         response = self.get_response(sort_by="date", query="timesSeen:>1t")
         assert response.status_code == 400
         assert response.status_code == 400
-        assert "Invalid format for numeric field" in response.data["detail"]
+        assert "Invalid number" in response.data["detail"]
 
 
     def test_valid_numeric_query(self):
     def test_valid_numeric_query(self):
         now = timezone.now()
         now = timezone.now()

+ 1 - 1
tests/snuba/api/endpoints/test_project_group_index.py

@@ -69,7 +69,7 @@ class GroupListTest(APITestCase, SnubaTestCase):
 
 
         response = self.client.get(f"{self.path}?sort_by=date&query=timesSeen:>1t", format="json")
         response = self.client.get(f"{self.path}?sort_by=date&query=timesSeen:>1t", format="json")
         assert response.status_code == 400
         assert response.status_code == 400
-        assert "could not" in response.data["detail"]
+        assert "Error parsing search query" in response.data["detail"]
 
 
     def test_simple_pagination(self):
     def test_simple_pagination(self):
         event1 = self.store_event(
         event1 = self.store_event(