diff --git a/swh/core/api/__init__.py b/swh/core/api/__init__.py --- a/swh/core/api/__init__.py +++ b/swh/core/api/__init__.py @@ -24,6 +24,7 @@ from deprecated import deprecated from flask import Flask, Request, Response, abort, request import requests +import sentry_sdk from werkzeug.exceptions import HTTPException from .negotiation import Formatter as FormatterBase @@ -415,7 +416,25 @@ def error_handler(exception, encoder, status_code=500): + """Error handler to be registered using flask's error-handling decorator + ``app.errorhandler``. + + This is used for exceptions that are expected in the normal execution flow of the + RPC-ed API, in which case the status code should be set to a value in the 4xx range, + as well as for exceptions that are unexpected (generally, a bare + :class:`Exception`), and for which the status code should be kept in the 5xx class. + + This function only captures exceptions as sentry errors if the status code is in the + 5xx range, as "expected exceptions" in the 4xx range are more likely to be handled + on the client side. + + """ logging.exception(exception) + + status_class = status_code // 100 + if status_class == 5: + sentry_sdk.capture_exception(exception) + response = encoder(exception_to_dict(exception)) if isinstance(exception, HTTPException): response.status_code = exception.code diff --git a/swh/core/api/tests/test_rpc_client_server.py b/swh/core/api/tests/test_rpc_client_server.py --- a/swh/core/api/tests/test_rpc_client_server.py +++ b/swh/core/api/tests/test_rpc_client_server.py @@ -15,6 +15,10 @@ ) +class ExpectedException(Exception): + """Another exception class to distinguish error handlers""" + + # this class is used on the server part class RPCTest: @remote_api_endpoint("endpoint_url") @@ -34,6 +38,10 @@ def raise_exception_exc_arg(self): raise Exception(Exception("error")) + @remote_api_endpoint("raises_expectedexc") + def raise_expectedexc(self): + raise ExpectedException("that was expected") + # this class is used on the client part. We cannot inherit from RPCTest # because the automagic metaclass based code that generates the RPCClient @@ -58,6 +66,10 @@ def raise_typeerror(self): return "data" + @remote_api_endpoint("raises_expectedexc") + def raise_expectedexc(self): + return "nothing" + class RPCTestClient(RPCClient): backend_class = RPCTest2 @@ -69,6 +81,10 @@ # which is defined in swh/core/pytest_plugin.py application = RPCServerApp("testapp", backend_class=RPCTest) + @application.errorhandler(ExpectedException) + def my_expected_error_handler(exception): + return error_handler(exception, encode_data_server, status_code=400) + @application.errorhandler(Exception) def my_error_handler(exception): return error_handler(exception, encode_data_server) @@ -109,10 +125,14 @@ assert res == "egg" -def test_api_typeerror(swh_rpc_client): +def test_api_typeerror(swh_rpc_client, mocker): + mocked_capture_exception = mocker.patch("swh.core.api.sentry_sdk.capture_exception") + with pytest.raises(RemoteException) as exc_info: swh_rpc_client.raise_typeerror() + assert mocked_capture_exception.called_once_with(TypeError("Did I pass through?")) + assert exc_info.value.args[0]["type"] == "TypeError" assert exc_info.value.args[0]["args"] == ["Did I pass through?"] assert ( @@ -128,3 +148,15 @@ assert exc_info.value.args[0]["type"] == "Exception" assert type(exc_info.value.args[0]["args"][0]) == Exception assert str(exc_info.value.args[0]["args"][0]) == "error" + + +def test_api_expected_exception_no_sentry_capture(swh_rpc_client, mocker): + mocked_capture_exception = mocker.patch("swh.core.api.sentry_sdk.capture_exception") + + with pytest.raises(RemoteException) as exc_info: + swh_rpc_client.raise_expectedexc() + + assert not mocked_capture_exception.called + + assert exc_info.value.args[0]["type"] == "ExpectedException" + assert exc_info.value.args[0]["args"] == ["that was expected"]