From 8b9011dff8c8c55c7b189dc0d30a45a8c12b0d4d Mon Sep 17 00:00:00 2001 From: PIG208 <359101898@qq.com> Date: Sun, 4 Jul 2021 16:00:55 +0800 Subject: [PATCH] json_error: Completely remove json_error. This completes the migration from `return json_error` to `raise JsonableError`. --- docs/translating/internationalization.md | 5 ++--- tools/linter_lib/custom_check.py | 12 ------------ zerver/lib/request.py | 2 +- zerver/lib/response.py | 4 ---- zerver/middleware.py | 4 ++-- zerver/tests/test_subs.py | 4 ++-- zerver/views/streams.py | 2 +- 7 files changed, 8 insertions(+), 25 deletions(-) diff --git a/docs/translating/internationalization.md b/docs/translating/internationalization.md index 08c7990225..8fdaeec30d 100644 --- a/docs/translating/internationalization.md +++ b/docs/translating/internationalization.md @@ -170,12 +170,11 @@ from django.utils.translation import gettext as _ ``` Zulip expects all the error messages to be translatable as well. To -ensure this, the error message passed to `json_error` and -`JsonableError` should always be a literal string enclosed by `_()` +ensure this, the error message passed to `JsonableError` +should always be a literal string enclosed by `_()` function, e.g.: ``` -json_error(_('English text')) JsonableError(_('English text')) ``` diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 3ecfea0784..9ff34622a2 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -319,18 +319,6 @@ python_rules = RuleList( "good_lines": ["return json_success()"], "bad_lines": ["return json_success({})"], }, - { - "pattern": r"\Wjson_error\(_\(?\w+\)", - "exclude": {"zerver/tests", "zerver/views/development/"}, - "description": "Argument to json_error should be a literal string enclosed by _()", - "good_lines": ['return json_error(_("string"))'], - "bad_lines": ["return json_error(_variable)", "return json_error(_(variable))"], - }, - { - "pattern": r"""\Wjson_error\(['"].+[),]$""", - "exclude": {"zerver/tests", "zerver/views/development/"}, - "description": "Argument to json_error should be a literal string enclosed by _()", - }, # To avoid JsonableError(_variable) and JsonableError(_(variable)) { "pattern": r"\WJsonableError\(_\(?\w.+\)", diff --git a/zerver/lib/request.py b/zerver/lib/request.py index e3ca34cc23..9f1dc0ac41 100644 --- a/zerver/lib/request.py +++ b/zerver/lib/request.py @@ -272,7 +272,7 @@ arguments_map: Dict[str, List[str]] = defaultdict(list) # the default parameter values used by has_request_variables. # # Note that this can't be used in helper functions which are not -# expected to call json_error or json_success, as it uses json_error +# expected to call json_success or raise JsonableError, as it uses JsonableError # internally when it encounters an error def has_request_variables(view_func: ViewFuncT) -> ViewFuncT: num_params = view_func.__code__.co_argcount diff --git a/zerver/lib/response.py b/zerver/lib/response.py index 5e5a400051..512d79b686 100644 --- a/zerver/lib/response.py +++ b/zerver/lib/response.py @@ -80,7 +80,3 @@ def json_response_from_error(exception: JsonableError) -> HttpResponse: response[header] = value return response - - -def json_error(msg: str, data: Mapping[str, Any] = {}, status: int = 400) -> HttpResponse: - return json_response(res_type="error", msg=msg, data=data, status=status) diff --git a/zerver/middleware.py b/zerver/middleware.py index 7d3b19d8b1..89f7f9037c 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -39,7 +39,7 @@ from zerver.lib.html_to_text import get_content_description from zerver.lib.markdown import get_markdown_requests, get_markdown_time from zerver.lib.rate_limiter import RateLimitResult from zerver.lib.request import set_request, unset_request -from zerver.lib.response import json_error, json_response_from_error, json_unauthorized +from zerver.lib.response import json_response, json_response_from_error, json_unauthorized from zerver.lib.subdomains import get_subdomain from zerver.lib.types import ViewFuncT from zerver.lib.user_agent import parse_user_agent @@ -448,7 +448,7 @@ class JsonErrorHandler(MiddlewareMixin): capture_exception(exception) json_error_logger = logging.getLogger("zerver.middleware.json_error_handler") json_error_logger.error(traceback.format_exc(), extra=dict(request=request)) - return json_error(_("Internal server error"), status=500) + return json_response(res_type="error", msg=_("Internal server error"), status=500) return None diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index a56cd4884b..f74792c97d 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -45,7 +45,7 @@ from zerver.lib.actions import ( validate_user_access_to_subscribers_helper, ) from zerver.lib.message import aggregate_unread_data, get_raw_unread_data -from zerver.lib.response import json_error, json_success +from zerver.lib.response import json_success from zerver.lib.stream_subscription import ( get_active_subscriptions_for_stream_id, num_subscribers_for_stream_id, @@ -2792,7 +2792,7 @@ class SubscriptionRestApiTest(ZulipTestCase): return json_success() def thunk2() -> HttpResponse: - return json_error("random failure") + raise JsonableError("random failure") with self.assertRaises(JsonableError): compose_views([thunk1, thunk2]) diff --git a/zerver/views/streams.py b/zerver/views/streams.py index e1938af20f..7c839be96f 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -371,7 +371,7 @@ def compose_views(thunks: List[Callable[[], HttpResponse]]) -> HttpResponse: for thunk in thunks: response = thunk() if response.status_code != 200: - raise JsonableError(response.content) + raise JsonableError(response.content) # nocoverage json_dict.update(orjson.loads(response.content)) return json_success(json_dict)