Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Invalid queries returning 5xx errors #286

Closed
jcscottiii opened this issue May 15, 2024 · 7 comments · Fixed by #1028
Closed

[Bug] Invalid queries returning 5xx errors #286

jcscottiii opened this issue May 15, 2024 · 7 comments · Fixed by #1028
Assignees
Labels
bug Something isn't working go Pull requests that update Go code hacktoberfest

Comments

@jcscottiii
Copy link
Collaborator

Expected behavior

  • Go to the home page
  • Type an incomplete search term. Example: available_on without the colon and value
  • Get the 400 page

Actual behavior

  • Go to the home page
  • Type an incomplete search term. Example: available_on without the colon and value
  • Get the 500 page

Other notes:

  • This was found by looking at the logs
    image
  • Typing the term with the colon (available_on:) returns a 400 error like expected
@jcscottiii jcscottiii added the bug Something isn't working label May 15, 2024
@jcscottiii
Copy link
Collaborator Author

jcscottiii commented May 15, 2024

It is likely getting caught in the ANY_VALUE term

May need to change the grammar to be something like this:

search_criteria:
    generic_search_term
-       | ANY_VALUE; // Default to ANY_VALUE search without "name:" prefix.
+      | ('name:' ANY_VALUE);  // Implicit name search for ANY_VALUE

@jcscottiii jcscottiii added the go Pull requests that update Go code label May 23, 2024
@jcscottiii jcscottiii moved this to Current Quarter in webstatus.dev May 28, 2024
@jcscottiii jcscottiii changed the title [Bug] Query without colon returns 500 [Bug] Invalid queries returning 5xx errors Jun 12, 2024
@jcscottiii
Copy link
Collaborator Author

jcscottiii commented Jun 12, 2024

From the Jun 11, 2024 error report

Here are some other invalid queries that should return 4xx instead of 5xx

For the first two queries, there was a trailing -. We need a new way to detect if the - is by itself then return a 400

The last two queries are similar to the original report where the user submitted available_on without the colon. Since this is happening more, I wonder if there is something we can do on the frontend to prevent the users from thinking available_on is okay to submit by itself.

@jcscottiii
Copy link
Collaborator Author

Definition of done for this bug:

  • Modify the grammar and run make gen to generate the generated files
  • Make any necessary changes to the implemented interface
  • Add test cases to the bad input test to catch the above queries

@past
Copy link
Collaborator

past commented Jun 13, 2024

If this is not urgent I can look into it in the next few days/weeks.

@jcscottiii jcscottiii moved this from 2024-Q2 to 2024-Q3 in webstatus.dev Jul 2, 2024
@jcscottiii jcscottiii moved this from 2024-Q3 to 2024-Q4 in webstatus.dev Sep 30, 2024
@jcscottiii
Copy link
Collaborator Author

From Oct 21 report. Queries that should be returning 4xx instead of 5xx

@jcscottiii
Copy link
Collaborator Author

From Nov 20 report:

Query: name:"has()" OR name:light-dark

@jcscottiii
Copy link
Collaborator Author

jcscottiii commented Jan 2, 2025

New report:

image

A few things happening:

  1. The colon was not encoded for some of them (and the date is the wrong format. M-D-YYYY)
  1. Some queries did not do the date range and instead only provided a single date

@jcscottiii jcscottiii self-assigned this Jan 2, 2025
jcscottiii added a commit that referenced this issue Jan 2, 2025
When a string is missing the colon or value for a term, the parser
still reaches the visitor method with a nil value node. This adds nil
checks to handle these cases and returns an error.

Test cases have been added to address the reported issues in #286.

Possible future improvement: Explore grammar modifications to prevent
these invalid inputs at the parser level.
github-merge-queue bot pushed a commit that referenced this issue Jan 3, 2025
)

* Handle missing values in grammar terms with nil checks in visitor

When a string is missing the colon or value for a term, the parser
still reaches the visitor method with a nil value node. This adds nil
checks to handle these cases and returns an error.

Test cases have been added to address the reported issues in #286.

Possible future improvement: Explore grammar modifications to prevent
these invalid inputs at the parser level.

* address feedback
@github-project-automation github-project-automation bot moved this from 2024-Q4 to Done in webstatus.dev Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code hacktoberfest
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants