diff --git a/config/staging.yml b/config/staging.yml --- a/config/staging.yml +++ b/config/staging.yml @@ -6,6 +6,6 @@ cls: remote url: http://webapp.internal.staging.swh.network:5010 -debug: yes +debug: no server-type: wsgi diff --git a/swh/graphql/errors/__init__.py b/swh/graphql/errors/__init__.py --- a/swh/graphql/errors/__init__.py +++ b/swh/graphql/errors/__init__.py @@ -3,7 +3,12 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -from .errors import ObjectNotFoundError, PaginationError +from .errors import InvalidInputError, ObjectNotFoundError, PaginationError from .handlers import format_error -__all__ = ["ObjectNotFoundError", "PaginationError", "format_error"] +__all__ = [ + "ObjectNotFoundError", + "PaginationError", + "InvalidInputError", + "format_error", +] diff --git a/swh/graphql/errors/errors.py b/swh/graphql/errors/errors.py --- a/swh/graphql/errors/errors.py +++ b/swh/graphql/errors/errors.py @@ -16,3 +16,7 @@ def __init__(self, message, errors=None): # FIXME, log this error super().__init__(f"{self.msg}: {message}") + + +class InvalidInputError(Exception): + """ """ diff --git a/swh/graphql/errors/handlers.py b/swh/graphql/errors/handlers.py --- a/swh/graphql/errors/handlers.py +++ b/swh/graphql/errors/handlers.py @@ -3,11 +3,26 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +from ariadne import format_error as original_format_error +from graphql import GraphQLError -def format_error(error) -> dict: +from .errors import InvalidInputError, ObjectNotFoundError, PaginationError + + +def format_error(error: GraphQLError, debug: bool = False): """ Response error formatting """ + original_format = original_format_error(error, debug) + if debug: + # If debug is enabled, reuse Ariadne's formatting logic with stack trace + return original_format + + expected_errors = [ObjectNotFoundError, PaginationError, InvalidInputError] formatted = error.formatted - formatted["message"] = "Unknown error" + formatted["message"] = error.message + if error.original_error.__class__ not in expected_errors: + # a crash, send original_format to sentry (with stack trace) + pass + # log the original_format to kibana (with stack trace) return formatted diff --git a/swh/graphql/resolvers/scalars.py b/swh/graphql/resolvers/scalars.py --- a/swh/graphql/resolvers/scalars.py +++ b/swh/graphql/resolvers/scalars.py @@ -7,6 +7,7 @@ from ariadne import ScalarType +from swh.graphql.errors import InvalidInputError from swh.graphql.utils import utils from swh.model import hashutil from swh.model.model import TimestampWithTimezone @@ -52,9 +53,9 @@ hash_value = hashutil.hash_to_bytes(hash_string) except ValueError as e: # FIXME, log this error - raise AttributeError("Invalid content checksum", e) + raise InvalidInputError("Invalid content checksum", e) except Exception as e: # FIXME, log this error - raise AttributeError("Invalid content checksum", e) + raise InvalidInputError("Invalid content checksum", e) # FIXME, add validation for the hash_type return hash_type, hash_value diff --git a/swh/graphql/server.py b/swh/graphql/server.py --- a/swh/graphql/server.py +++ b/swh/graphql/server.py @@ -63,6 +63,7 @@ configuration path to load. """ from .app import schema + from .errors.handlers import format_error global graphql_cfg @@ -77,12 +78,14 @@ # Enable cors in the asgi version application = CORSMiddleware( - GraphQL(schema), + GraphQL(schema, debug=graphql_cfg["debug"], error_formatter=format_error), allow_origins=["*"], allow_methods=("GET", "POST", "OPTIONS"), ) else: from ariadne.wsgi import GraphQL - application = GraphQL(schema) + application = GraphQL( + schema, debug=graphql_cfg["debug"], error_formatter=format_error + ) return application diff --git a/swh/graphql/tests/functional/test_error_handling.py b/swh/graphql/tests/functional/test_error_handling.py new file mode 100644 --- /dev/null +++ b/swh/graphql/tests/functional/test_error_handling.py @@ -0,0 +1,22 @@ +# Copyright (C) 2022 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU General Public License version 3, or any later version +# See top-level LICENSE file for more information + + +def test_error_with_debug(): + pass + + +def test_error_without_debug(): + pass + + +def test_send_logger_and_not_to_sentry(): + # FIXME, test after the sentry integration + pass + + +def test_send_to_sentry(): + # FIXME, test after the sentry integration + pass