Skip to content

fix(parser)!: throw query kit exceptions for bad values and sort directions - #119

Open
pdevito3 wants to merge 1 commit into
mainfrom
fm/qk-breaking-querykit-exceptions
Open

pdevito3 wants to merge 1 commit into
mainfrom
fm/qk-breaking-querykit-exceptions

Conversation

@pdevito3

Copy link
Copy Markdown
Owner

Summary

This PR makes bad values, bad sort directions, and unknown logical operators throw QueryKit exceptions. This is a breaking change. It is one of the six breaking changes that were removed from #114 so that main stays compatible with v1.14.2.

Status: for later consideration. Do not merge this PR now. The captain decides on each breaking change separately. If this change is accepted, it needs a major version.

Main already accepts more than one space before a sort direction (Age desc). This PR adds only the three exception type changes.

Old behavior (v1.14.2 and main)

  • A value that does not convert to the property type throws FormatException or OverflowException from ApplyQueryKitFilter.
  • An invalid sort direction throws ArgumentException.
  • An unknown logical operator throws System.Exception.

New behavior

  • A value that does not convert throws ParsingException. The original exception is the inner exception.
  • An invalid sort direction throws QueryKitParsingException.
  • An unknown logical operator throws QueryKitParsingException.

All three new types derive from QueryKitException.

Example

Proof from the verify-querykit harness. Each row gives the same result in memory and on Postgres.

Input main This PR
filter Rating == "abc" FormatException ParsingException, inner message "The input string 'abc' was not in a correct format."
filter Rating == 99999999999 OverflowException ParsingException, inner message "Value was either too large or too small for an Int32."
sort Rating sideways ArgumentException QueryKitParsingException: "Invalid direction: sideways. Allowed values are 'asc' and 'desc'."

Justification

The README says that QueryKitException is the base class of all QueryKit exceptions, and that a consumer can catch it. In v1.14.2, some bad filters escape this catch as FormatException, OverflowException, ArgumentException, or System.Exception. As a result, the API returns a 500 for a client error.

After the change, one catch (QueryKitException) handles every bad filter and every bad sort. The break is limited to a consumer that catches the old specific types. That consumer can catch QueryKitException instead. The inner exception keeps the original details.

Migration

Catch QueryKitException, not FormatException, OverflowException, ArgumentException, or Exception.

Tests

  • New unit tests: invalid_value_throws_parsing_exception, invalid_sort_direction_throws_query_kit_parsing_exception, and unknown_logical_operator_throws_query_kit_parsing_exception.
  • New integration test: invalid_value_throws_parsing_exception.
  • dotnet test: 358 unit tests and 272 integration tests pass.

…ctions

A value that does not convert to the property type (for example `Age == "abc"`, `Rating > abc`, or an int overflow) threw FormatException or OverflowException. It now throws ParsingException, with the original exception as the inner exception.

An invalid sort direction now throws QueryKitParsingException instead of ArgumentException. An unknown logical operator throws QueryKitParsingException instead of a plain Exception. All of these types derive from QueryKitException.

BREAKING CHANGE: a bad filter value throws ParsingException, not FormatException or OverflowException. An invalid sort direction throws QueryKitParsingException, not ArgumentException. An unknown logical operator throws QueryKitParsingException, not System.Exception. Catch QueryKitException to handle all of them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant