Skip to content

Commit

Permalink
Merge pull request #106 from jsangmeister/more-logging
Browse files Browse the repository at this point in the history
Add better filter error description
  • Loading branch information
jsangmeister committed Jan 7, 2021
2 parents 8d3c7b0 + 624585c commit e20f2cc
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 9 deletions.
4 changes: 4 additions & 0 deletions dc.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ services:
- postgresql
networks:
- postgresql
stdin_open: true
tty: true
writer:
image: openslides-datastore-writer-dev
volumes:
Expand All @@ -33,6 +35,8 @@ services:
networks:
- postgresql
- redis
stdin_open: true
tty: true
postgresql:
image: postgres:11
env_file: database.env
Expand Down
6 changes: 5 additions & 1 deletion reader/reader/flask_frontend/json_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from shared.di import injector
from shared.flask_frontend import InvalidRequest, dev_only_route
from shared.typing import JSON
from shared.util import BadCodingError
from shared.util import BadCodingError, logger

from .routes import Route, route_configurations

Expand All @@ -25,9 +25,13 @@ def handle_request(self, route: Route, data: JSON) -> Dict:
except KeyError:
raise BadCodingError("Invalid route metadata: " + route)

logger.info(f"{route.upper()}-request: {data}")

try:
request_data = route_configuration.schema(data)
except fastjsonschema.JsonSchemaException as e:
if route_configuration.schema_error_handler:
route_configuration.schema_error_handler(e)
raise InvalidRequest(e.message)

try:
Expand Down
39 changes: 32 additions & 7 deletions reader/reader/flask_frontend/routes.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from dataclasses import dataclass
from enum import Enum
from typing import Callable, Dict, Type
from typing import Callable, Dict, Optional, Type

import fastjsonschema

Expand All @@ -13,7 +13,7 @@
GetRequest,
MinMaxRequest,
)
from shared.flask_frontend import unify_urls
from shared.flask_frontend import InvalidRequest, unify_urls
from shared.postgresql_backend.sql_query_helper import VALID_AGGREGATE_CAST_TARGETS
from shared.util import DeletedModelsBehaviour

Expand Down Expand Up @@ -216,10 +216,19 @@ def URL(self):
)


def handle_filter_schema_error(e: fastjsonschema.JsonSchemaException) -> None:
if e.rule == "anyOf":
# we only use anyOf for filters, so an invalid filter definition was given
raise InvalidRequest(f"Invalid filter definition: {e.value}")


@dataclass
class RouteConfiguration:
schema: Callable
request_class: Type
schema_error_handler: Optional[
Callable[[fastjsonschema.JsonSchemaException], None]
] = None
dev_only: bool = False


Expand All @@ -235,13 +244,29 @@ class RouteConfiguration:
Route.GET_EVERYTHING: RouteConfiguration(
schema=get_everything_schema, request_class=GetEverythingRequest, dev_only=True
),
Route.FILTER: RouteConfiguration(schema=filter_schema, request_class=FilterRequest),
Route.FILTER: RouteConfiguration(
schema=filter_schema,
request_class=FilterRequest,
schema_error_handler=handle_filter_schema_error,
),
Route.EXISTS: RouteConfiguration(
schema=aggregate_schema, request_class=AggregateRequest
schema=aggregate_schema,
request_class=AggregateRequest,
schema_error_handler=handle_filter_schema_error,
),
Route.COUNT: RouteConfiguration(
schema=aggregate_schema, request_class=AggregateRequest
schema=aggregate_schema,
request_class=AggregateRequest,
schema_error_handler=handle_filter_schema_error,
),
Route.MIN: RouteConfiguration(
schema=minmax_schema,
request_class=MinMaxRequest,
schema_error_handler=handle_filter_schema_error,
),
Route.MAX: RouteConfiguration(
schema=minmax_schema,
request_class=MinMaxRequest,
schema_error_handler=handle_filter_schema_error,
),
Route.MIN: RouteConfiguration(schema=minmax_schema, request_class=MinMaxRequest),
Route.MAX: RouteConfiguration(schema=minmax_schema, request_class=MinMaxRequest),
}
2 changes: 1 addition & 1 deletion scripts/make-targets
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
MAKE_TARGETS_SHARED=build-tests run-cleanup run-cleanup-with-update setup-docker-compose run-tests-no-down run-tests run-bash run-tests-interactive run-coverage run-travis-no-down run-travis
MAKE_TARGETS_MODULES=$(MAKE_TARGETS_SHARED) run-integration-unit-tests run-coverage-integration-unit run-integration-unit-tests-interactive run-system-tests build-dev run-dev run-dev-verbose build-prod run-prod run-prod-verbose stop-prod build run run-verbose stop
MAKE_TARGETS_MODULES=$(MAKE_TARGETS_SHARED) run-integration-unit-tests run-coverage-integration-unit run-integration-unit-tests-interactive run-system-tests build-dev run-dev run-dev-verbose stop-dev build-prod run-prod run-prod-verbose stop-prod build run run-verbose stop
export
8 changes: 8 additions & 0 deletions shared/shared/flask_frontend/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@

from werkzeug.exceptions import default_exceptions

from shared.di import injector
from shared.postgresql_backend.connection_handler import DatabaseError
from shared.services import EnvironmentService
from shared.util import (
InvalidFormat,
ModelDoesNotExist,
ModelExists,
ModelLocked,
ModelNotDeleted,
logger,
)

from .json_response import JsonResponse
Expand Down Expand Up @@ -72,6 +75,11 @@ def wrapper(*args, **kwargs):
except Exception as e:
print(e, type(e))
raise e

env_service = injector.get(EnvironmentService)
if env_service.is_dev_mode():
logger.debug(f"HTTP error 400: {error_dict}")

return {"error": error_dict}, 400

return wrapper
Expand Down

0 comments on commit e20f2cc

Please sign in to comment.