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 @@ -7,6 +7,11 @@ class ObjectNotFoundError(Exception): """ """ + msg: str = "Object error" + + def __init__(self, message, errors=None): + super().__init__(f"{self.msg}: {message}") + class PaginationError(Exception): """ """ @@ -14,5 +19,13 @@ msg: str = "Pagination error" def __init__(self, message, errors=None): - # FIXME, log this error + super().__init__(f"{self.msg}: {message}") + + +class InvalidInputError(Exception): + """ """ + + msg: str = "Input error" + + def __init__(self, message, errors=None): super().__init__(f"{self.msg}: {message}") 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,27 @@ # 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 +import sentry_sdk -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 type(error.original_error) not in expected_errors: + # a crash, send to sentry + sentry_sdk.capture_exception(error) + # FIXME 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 @@ -51,10 +52,7 @@ hash_type, hash_string = value.split(":") hash_value = hashutil.hash_to_bytes(hash_string) except ValueError as e: - # FIXME, log this error - raise AttributeError("Invalid content checksum", e) - except Exception as e: - # FIXME, log this error - raise AttributeError("Invalid content checksum", e) - # FIXME, add validation for the hash_type + raise InvalidInputError("Invalid content checksum", e) + if hash_type not in hashutil.ALGORITHMS: + raise InvalidInputError("Invalid hash algorithm") 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_content.py b/swh/graphql/tests/functional/test_content.py --- a/swh/graphql/tests/functional/test_content.py +++ b/swh/graphql/tests/functional/test_content.py @@ -117,7 +117,21 @@ ) # API will throw an error in case of an invalid content hash assert len(errors) == 1 - assert "Invalid content checksum" in errors[0]["message"] + assert "Input error: Invalid content checksum" in errors[0]["message"] + + +def test_get_content_with_invalid_hash_algorithm(client): + content = get_contents()[0] + query_str = """ + { + contentByHash(checksums: ["test:%s"]) { + swhid + } + } + """ + errors = utils.get_error_response(client, query_str % content.sha1.hex()) + assert len(errors) == 1 + assert "Input error: Invalid hash algorithm" in errors[0]["message"] def test_get_content_as_target(client): diff --git a/swh/graphql/tests/functional/utils.py b/swh/graphql/tests/functional/utils.py --- a/swh/graphql/tests/functional/utils.py +++ b/swh/graphql/tests/functional/utils.py @@ -22,7 +22,7 @@ data, errors = get_query_response(client, query_str) assert data[obj_type] is None assert len(errors) == 1 - assert errors[0]["message"] == "Requested object is not available" + assert errors[0]["message"] == "Object error: Requested object is not available" def get_error_response(client, query_str: str, error_code: int = 400) -> Dict: diff --git a/swh/graphql/tests/unit/errors/test_errors.py b/swh/graphql/tests/unit/errors/test_errors.py new file mode 100644 --- /dev/null +++ b/swh/graphql/tests/unit/errors/test_errors.py @@ -0,0 +1,53 @@ +# 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 + +from graphql import GraphQLError +import pytest +import sentry_sdk + +from swh.graphql import errors + + +def test_errors(): + err = errors.ObjectNotFoundError("test error") + assert str(err) == "Object error: test error" + + err = errors.PaginationError("test error") + assert str(err) == "Pagination error: test error" + + err = errors.InvalidInputError("test error") + assert str(err) == "Input error: test error" + + +def test_format_error_with_debug(): + err = GraphQLError("test error") + response = errors.format_error(err, debug=True) + assert "extensions" in response + + +def test_format_error_without_debug(): + err = GraphQLError("test error") + response = errors.format_error(err) + assert "extensions" not in response + + +def test_format_error_sent_to_sentry(mocker): + mocked_senty_call = mocker.patch.object(sentry_sdk, "capture_exception") + err = GraphQLError("test error") + err.original_error = NameError("test error") # not an expected error + errors.format_error(err) + mocked_senty_call.assert_called_once_with(err) + + +@pytest.mark.parametrize( + "error", + [errors.ObjectNotFoundError, errors.PaginationError, errors.InvalidInputError], +) +def test_format_error_skip_sentry(mocker, error): + mocked_senty_call = mocker.patch.object(sentry_sdk, "capture_exception") + err = GraphQLError("test error") + err.original_error = error("test error") + errors.format_error(err) + mocked_senty_call.assert_not_called