Skip to content

Conversation

@seantleonard
Copy link
Contributor

@seantleonard seantleonard commented Sep 27, 2022

Why make this change?

Closes #835

  • When submitting a REST request on the DELETE endpoint, improper handling of the raised exception when a type fails to be parsed to the expected type results in an HTTP 500 error.
    • e.g. when a primary key value's type in a REST request does not match AND cannot cast to the type in the underlying database column.

What is this change?

  • Adds a Try/Catch block in the DeleteSqlQueryStructure code path to match the other 5 references of the parsing method at fault. The Catch block now properly catches the ArgumentException raised and raises a new DataApiBuilderException which signals the runtime to return an HTTP 400 Bad Request error.

How was this tested?

  • Integration Test coverage added for Delete REST path AND Insert/Put/Patch/Find paths for full coverage to prevent regressions. Each operation has a different code path which should properly handle cast conversion of the PK.

Sample Request(s)

  • REST Request
    URL: DELETE /api/Book/id/{}
    Expected to fail with HTTP 400 Bad Request and NOT HTTP 5XX.

@seantleonard seantleonard linked an issue Sep 27, 2022 that may be closed by this pull request
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for checking GraphQL scenario.

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, after double checking 1 more question.

@seantleonard seantleonard requested a review from jarupatj October 19, 2022 18:41
@seantleonard seantleonard merged commit 3954027 into main Oct 21, 2022
@seantleonard seantleonard deleted the dev/seleonar/typeConversionErrorHandlingFix branch October 21, 2022 18:40
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.

Invalid type conversion incorrectly triggers HTTP 500 Server error

4 participants