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

Catch certain errors in the API and return information about them as response #186

Merged
merged 20 commits into from
Oct 29, 2021

Conversation

matthiasschaub
Copy link
Collaborator

@matthiasschaub matthiasschaub commented Oct 3, 2021

Description

Per default errors which arise during validation of an API request (pydantic models) are caught by FastAPI and details are returned in the API response (ReqestValidationError). Cases covered by this are:

  • invalid set/combination of parameter
  • invalid bpolys (Valid GeoJSON and allowed Geometries)

This PR will extend the numbers of errors about which FastAPI will return details.
In particular it will add custom exception handlers to FastAPI (FastAPI docs). Thise will cover two more cases:

  • OhsomeApiError`: If status code of ohsome API response is 4xx or 5xx or if response is invalid even though status code is 200 (Happens if response is streamed)
  • SizeRestrictionError: bpolys exceeds geometry size limit

In addition this PR seeks to unify the response schema for errors raised during API request parameter validation (pydatic models) and those covered by the custom exception handlers. This is not as straightforward as it sounds since the ReqestValidationError contains information about multiple errors ("One exception will be raised regardless of the number of errors found, that ValidationError will contain information about all the errors and how they happened." (pydantic docs). In effect to each response only the error type is added.

Current response schema for custom errors:

{'detail': 'Input GeoJSON Object is too big. The area should be less than 100 km².'}

Current response schema for validation errors:

{
  "detail": [
    {
      "loc": [
        "body",
        "dataset"
      ],
      "msg": "value is not a valid enumeration member; permitted: 'regions'",
      "type": "type_error.enum",
      "ctx": {
        "enum_values": [
          "regions"
        ]
      }
    },
    {
      "loc": [
        "body",
        "layerName"
      ],
      "msg": "field required",
      "type": "value_error.missing"
    }
  ]
}

Changed response schema for custom errors:

{"detail": "message", "type": "OhsomeApiError"}

Changed response schema for custom errors:

{
  "detail": [
    {
      "loc": [
        "body",
        "dataset"
      ],
      "msg": "value is not a valid enumeration member; permitted: 'regions'",
      "type": "type_error.enum",
      "ctx": {
        "enum_values": [
          "regions"
        ]
      }
    },
    {
      "loc": [
        "body",
        "layerName"
      ],
      "msg": "field required",
      "type": "value_error.missing"
    }
  ],
  "type": "RequestValidationError"
}

Open Issues

This should probably be its own PR -> #207:

  • If the input is a GeoJSON FeatureCollection no information will passed down to the API for which feature the error ocured.
    • Possible solution is to use feature.id in each error message.

Corresponding issue

Closes #107

Related #103

Checklist

  • I have updated my branch to main (e.g. through git rebase main)
  • My code follows the style guide and was checked with pre-commit before committing
  • I have commented my code
  • I have added sufficient unit and integration tests
    • Missing tests for reports
  • I have updated the CHANGELOG.md

@matthiasschaub matthiasschaub added the comments welcome Indicates that the creator of this PR is open for early review comments label Oct 4, 2021
@joker234 joker234 added this to the Release 0.7.0 milestone Oct 6, 2021
@matthiasschaub matthiasschaub marked this pull request as ready for review October 24, 2021 15:25
@matthiasschaub matthiasschaub force-pushed the api-error-msg branch 5 times, most recently from 2b1b1c9 to 91c5c39 Compare October 26, 2021 05:42
@matthiasschaub matthiasschaub removed the comments welcome Indicates that the creator of this PR is open for early review comments label Oct 27, 2021
@matthiasschaub matthiasschaub force-pushed the api-error-msg branch 2 times, most recently from 9e1c84a to 845d15d Compare October 29, 2021 04:33
Copy link
Member

@joker234 joker234 left a comment

Choose a reason for hiding this comment

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

👍 some minor remarks ;)

CHANGELOG.md Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/api/request_models.py Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/oqt.py Show resolved Hide resolved
parameter in pydantic request model.
when the input is a Feature Collection.
Either Feature id field if present or otherwise the enumeration variable.
geometry and return information about this error as API response.
FastAPIs `RequestValidationError` is a subclass of pydantics `ValidationError`.
Because of the usage of `@pydantic.validate_arguments` decorator
`ValidationError` needs to be specified in the FastAPI exception handler as well.
Refactor out AsyncMock class to utils.py because it is used in multiple
modules.
block is empty. This will also avoid SonarCloud error.
unittests directory instead of integrationtests directory.
joker234
joker234 previously approved these changes Oct 29, 2021
@matthiasschaub matthiasschaub merged commit c6ec4a1 into main Oct 29, 2021
@matthiasschaub matthiasschaub deleted the api-error-msg branch October 29, 2021 16:30
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.

API should return a error message describing what went wrong when request fails
2 participants