report_error: Remove API endpoint for client error reporting.

pyproject.toml

@@ -83,7 +83,6 @@ module = [
     "scrapy.*", # https://github.com/scrapy/scrapy/issues/4041
     "social_core.*",
     "social_django.*",
-    "sourcemap.*",
     "talon_core.*",
     "tlds.*",
     "twitter.*",

requirements/common.in

@@ -82,9 +82,6 @@ https://github.com/andersk/zoneinfo/archive/f9687abaea8453be1c8d0e21544bd557d65a
 # Needed for Redis
 redis
 
-# Needed to parse source maps for error reporting
-sourcemap
-
 # Tornado used for server->client push system
 tornado
 

requirements/dev.txt

@@ -2111,10 +2111,6 @@ soupsieve==2.4 \
     # via
     #   -r requirements/common.in
     #   beautifulsoup4
-sourcemap==0.2.1 \
-    --hash=sha256:be00a90185e7a16b87bbe62a68ffd5e38bc438ef4700806d9b90e44d8027787c \
-    --hash=sha256:c448a8c48f9482e522e4582106b0c641a83b5dbc7f13927b178848e3ea20967b
-    # via -r requirements/common.in
 sphinx==6.1.3 \
     --hash=sha256:0dac3b698538ffef41716cf97ba26c1c7788dba73ce6f150c1ff5b4720786dd2 \
     --hash=sha256:807d1cb3d6be87eb78a381c3e70ebd8d346b9a25f3753e9947e866b2786865fc

requirements/prod.txt

@@ -1467,10 +1467,6 @@ soupsieve==2.4 \
     # via
     #   -r requirements/common.in
     #   beautifulsoup4
-sourcemap==0.2.1 \
-    --hash=sha256:be00a90185e7a16b87bbe62a68ffd5e38bc438ef4700806d9b90e44d8027787c \
-    --hash=sha256:c448a8c48f9482e522e4582106b0c641a83b5dbc7f13927b178848e3ea20967b
-    # via -r requirements/common.in
 sqlalchemy==1.4.47 \
     --hash=sha256:03be6f3cb66e69fb3a09b5ea89d77e4bc942f3bf84b207dba84666a26799c166 \
     --hash=sha256:048509d7f3ac27b83ad82fd96a1ab90a34c8e906e4e09c8d677fc531d12c23c5 \

tools/test-backend

@@ -103,7 +103,6 @@ not_yet_fully_covered = [
     "zerver/lib/queue.py",
     "zerver/lib/sqlalchemy_utils.py",
     "zerver/lib/storage.py",
-    "zerver/lib/unminify.py",
     "zerver/lib/utils.py",
     "zerver/lib/zephyr.py",
     "zerver/lib/templates.py",

zerver/lib/unminify.py

@@ -1,72 +0,0 @@
-import os
-import re
-from typing import Dict, List, Optional
-
-import sourcemap
-
-from zerver.lib.pysa import mark_sanitized
-
-
-class SourceMap:
-    """Map (line, column) pairs from generated to source file."""
-
-    def __init__(self, sourcemap_dirs: List[str]) -> None:
-        self._dirs = sourcemap_dirs
-        self._indices: Dict[str, sourcemap.SourceMapDecoder] = {}
-
-    def _index_for(self, minified_src: str) -> Optional[sourcemap.SourceMapDecoder]:
-        """Return the source map index for minified_src, loading it if not
-        already loaded."""
-
-        # Prevent path traversal
-        assert ".." not in minified_src and "/" not in minified_src
-
-        if minified_src not in self._indices:
-            for source_dir in self._dirs:
-                filename = os.path.join(source_dir, minified_src + ".map")
-                if os.path.isfile(filename):
-                    # Use 'mark_sanitized' to force Pysa to ignore the fact that
-                    # 'filename' is user controlled. While putting user
-                    # controlled data into a filesystem operation is bad, in
-                    # this case it's benign because 'filename' can't traverse
-                    # directories outside of the pre-configured 'sourcemap_dirs'
-                    # (due to the above assertions) and will always end in
-                    # '.map'. Additionally, the result of this function is used
-                    # for error logging and not returned to the user, so
-                    # controlling the loaded file would not be useful to an
-                    # attacker.
-                    with open(mark_sanitized(filename)) as fp:
-                        self._indices[minified_src] = sourcemap.load(fp)
-                        break
-
-        return self._indices.get(minified_src)
-
-    def annotate_stacktrace(self, stacktrace: str) -> str:
-        out: str = ""
-        for ln in stacktrace.splitlines():
-            out += ln + "\n"
-            match = re.search(r"/webpack-bundles/([^:]+):(\d+):(\d+)", ln)
-            if match:
-                # Get the appropriate source map for the minified file.
-                minified_src = match.groups()[0]
-                index = self._index_for(minified_src)
-                if index is None:
-                    out += "       [Unable to look up in source map]\n"
-                    continue
-
-                gen_line, gen_col = list(map(int, match.groups()[1:3]))
-                # The sourcemap lib is 0-based, so subtract 1 from line and col.
-                try:
-                    result = index.lookup(line=gen_line - 1, column=gen_col - 1)
-                    display_src = result.src
-                    if display_src is not None:
-                        webpack_prefix = "webpack:///"
-                        if display_src.startswith(webpack_prefix):
-                            display_src = display_src[len(webpack_prefix) :]
-                        out += f"       = {display_src} line {result.src_line+1} column {result.src_col+1}\n"
-                except IndexError:
-                    out += "       [Unable to look up in source map]\n"
-
-            if ln.startswith("    at"):
-                out += "\n"
-        return out

zerver/middleware.py

@@ -112,10 +112,7 @@ def format_timedelta(timedelta: float) -> str:
 def is_slow_query(time_delta: float, path: str) -> bool:
     if time_delta < 1.2:
         return False
-    is_exempt = path in [
+    is_exempt = path == "/activity" or path.startswith(("/realm_activity/", "/user_activity/"))
-        "/activity",
-        "/json/report/error",
-    ] or path.startswith(("/realm_activity/", "/user_activity/"))
     if is_exempt:
         return time_delta >= 5
     if "webathena_kerberos" in path:

zerver/tests/test_middleware.py

@@ -31,7 +31,6 @@ class SlowQueryTest(ZulipTestCase):
         self.assertTrue(is_slow_query(2, "/some/random/url"))
         self.assertTrue(is_slow_query(5.1, "/activity"))
         self.assertFalse(is_slow_query(2, "/activity"))
-        self.assertFalse(is_slow_query(2, "/json/report/error"))
         self.assertFalse(is_slow_query(2, "/realm_activity/whatever"))
         self.assertFalse(is_slow_query(2, "/user_activity/whatever"))
         self.assertFalse(is_slow_query(9, "/accounts/webathena_kerberos_login/"))

zerver/tests/test_report.py

@@ -1,20 +1,10 @@
-from typing import Callable, ContextManager, Dict, List, Tuple
+from typing import Callable, ContextManager, List, Tuple
 from unittest import mock
 
-import orjson
-from django.test import override_settings
-
 from zerver.lib.test_classes import ZulipTestCase
-from zerver.lib.test_helpers import mock_queue_publish
 from zerver.lib.utils import statsd
 
 
-def fix_params(raw_params: Dict[str, object]) -> Dict[str, str]:
-    # A few of our few legacy endpoints need their
-    # individual parameters serialized as JSON.
-    return {k: orjson.dumps(v).decode() for k, v in raw_params.items()}
-
-
 class StatsMock:
     def __init__(self, settings: Callable[..., ContextManager[None]]) -> None:
         self.settings = settings
@@ -132,70 +122,6 @@ class TestReport(ZulipTestCase):
         ]
         self.assertEqual(stats_mock.func_calls, expected_calls)
 
-    @override_settings(BROWSER_ERROR_REPORTING=True)
-    def test_report_error(self) -> None:
-        user = self.example_user("hamlet")
-        self.login_user(user)
-        self.make_stream("errors", user.realm)
-
-        params = fix_params(
-            dict(
-                message="hello",
-                stacktrace="trace",
-                user_agent="agent",
-                href="href",
-                log="log",
-                more_info=dict(foo="bar", draft_content="**draft**"),
-            )
-        )
-
-        version_mock = mock.patch("zerver.views.report.ZULIP_VERSION", spec="1.2.3")
-        with mock_queue_publish("zerver.views.report.queue_json_publish") as m, version_mock:
-            result = self.client_post("/json/report/error", params)
-        self.assert_json_success(result)
-
-        report = m.call_args[0][1]["report"]
-        for k in set(params) - {"more_info"}:
-            self.assertEqual(report[k], params[k])
-
-        self.assertEqual(report["more_info"], dict(foo="bar", draft_content="'**xxxxx**'"))
-        self.assertEqual(report["user"]["user_email"], user.delivery_email)
-
-        # Test with no more_info
-        del params["more_info"]
-        with mock_queue_publish("zerver.views.report.queue_json_publish") as m, version_mock:
-            result = self.client_post("/json/report/error", params)
-        self.assert_json_success(result)
-
-        with self.settings(BROWSER_ERROR_REPORTING=False):
-            result = self.client_post("/json/report/error", params)
-        self.assert_json_success(result)
-
-        # If js_source_map is present, then the stack trace should be annotated.
-        # DEVELOPMENT=False and TEST_SUITE=False are necessary to ensure that
-        # js_source_map actually gets instantiated.
-        with self.settings(DEVELOPMENT=False, TEST_SUITE=False), mock.patch(
-            "zerver.lib.unminify.SourceMap.annotate_stacktrace"
-        ) as annotate, self.assertLogs(level="INFO") as info_logs, version_mock:
-            result = self.client_post("/json/report/error", params)
-        self.assert_json_success(result)
-        # fix_params (see above) adds quotes when JSON encoding.
-        annotate.assert_called_once_with('"trace"')
-        self.assertEqual(
-            info_logs.output, ["INFO:root:Processing traceback with type browser for None"]
-        )
-
-        # Now test without authentication.
-        self.logout()
-        with self.settings(DEVELOPMENT=False, TEST_SUITE=False), mock.patch(
-            "zerver.lib.unminify.SourceMap.annotate_stacktrace"
-        ) as annotate, self.assertLogs(level="INFO") as info_logs:
-            result = self.client_post("/json/report/error", params)
-        self.assert_json_success(result)
-        self.assertEqual(
-            info_logs.output, ["INFO:root:Processing traceback with type browser for None"]
-        )
-
     def test_report_csp_violations(self) -> None:
         fixture_data = self.fixture_data("csp_report.json")
         with self.assertLogs(level="WARNING") as warn_logs:

zerver/views/report.py

@@ -1,48 +1,26 @@
 # System documented in https://zulip.readthedocs.io/en/latest/subsystems/logging.html
 import logging
-from typing import Any, Mapping, Optional, Union
+from typing import Union
-from urllib.parse import SplitResult
 
-from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django.http import HttpRequest, HttpResponse
 from django.views.decorators.csrf import csrf_exempt
 from django.views.decorators.http import require_POST
 
-from version import ZULIP_VERSION
 from zerver.context_processors import get_valid_realm_from_request
 from zerver.decorator import human_users_only
-from zerver.lib.markdown import privacy_clean_markdown
-from zerver.lib.queue import queue_json_publish
 from zerver.lib.request import REQ, RequestNotes, has_request_variables
 from zerver.lib.response import json_success
-from zerver.lib.storage import static_path
-from zerver.lib.unminify import SourceMap
 from zerver.lib.utils import statsd, statsd_key
 from zerver.lib.validator import (
     WildValue,
     check_bool,
-    check_dict,
     check_string,
     to_non_negative_int,
     to_wild_value,
 )
 from zerver.models import UserProfile
 
-js_source_map: Optional[SourceMap] = None
-
-
-# Read the source map information for decoding JavaScript backtraces.
-def get_js_source_map() -> Optional[SourceMap]:
-    global js_source_map
-    if not js_source_map and not (settings.DEVELOPMENT or settings.TEST_SUITE):
-        js_source_map = SourceMap(
-            [
-                static_path("webpack-bundles"),
-            ]
-        )
-    return js_source_map
-
 
 @human_users_only
 @has_request_variables
@@ -117,72 +95,6 @@ def report_unnarrow_times(
     return json_success(request)
 
 
-@has_request_variables
-def report_error(
-    request: HttpRequest,
-    maybe_user_profile: Union[AnonymousUser, UserProfile],
-    message: str = REQ(),
-    stacktrace: str = REQ(),
-    user_agent: str = REQ(),
-    href: str = REQ(),
-    log: str = REQ(),
-    web_version: Optional[str] = REQ(default=None),
-    more_info: Mapping[str, Any] = REQ(json_validator=check_dict([]), default={}),
-) -> HttpResponse:
-    """Accepts an error report and stores in a queue for processing.  The
-    actual error reports are later handled by do_report_error"""
-    if not settings.BROWSER_ERROR_REPORTING:
-        return json_success(request)
-    more_info = dict(more_info)
-
-    js_source_map = get_js_source_map()
-    if js_source_map:
-        stacktrace = js_source_map.annotate_stacktrace(stacktrace)
-
-    server_version = str(ZULIP_VERSION)
-
-    # Get the IP address of the request
-    remote_ip = request.META["REMOTE_ADDR"]
-
-    # For the privacy of our users, we remove any actual text content
-    # in draft_content (from drafts rendering exceptions).  See the
-    # comment on privacy_clean_markdown for more details.
-    if more_info.get("draft_content"):
-        more_info["draft_content"] = privacy_clean_markdown(more_info["draft_content"])
-
-    if maybe_user_profile.is_authenticated:
-        user = {
-            "user_email": maybe_user_profile.delivery_email,
-            "user_full_name": maybe_user_profile.full_name,
-            "user_role": maybe_user_profile.get_role_name(),
-        }
-    else:
-        user = None
-
-    queue_json_publish(
-        "error_reports",
-        dict(
-            type="browser",
-            report=dict(
-                host=SplitResult("", request.get_host(), "", "", "").hostname,
-                ip_address=remote_ip,
-                user=user,
-                server_path=settings.DEPLOY_ROOT,
-                server_version=server_version,
-                web_version=web_version,
-                user_agent=user_agent,
-                href=href,
-                message=message,
-                stacktrace=stacktrace,
-                log=log,
-                more_info=more_info,
-            ),
-        ),
-    )
-
-    return json_success(request)
-
-
 @csrf_exempt
 @require_POST
 @has_request_variables

zproject/default_settings.py

@@ -126,7 +126,6 @@ EMAIL_GATEWAY_EXTRA_PATTERN_HACK: Optional[str] = None
 
 # Error reporting
 ERROR_REPORTING = True
-BROWSER_ERROR_REPORTING = False
 LOGGING_SHOW_MODULE = False
 LOGGING_SHOW_PID = False
 

zproject/prod_settings_template.py

@@ -652,8 +652,6 @@ SOCIAL_AUTH_SAML_SUPPORT_CONTACT = {
 ## Controls whether or not error reports (tracebacks) are emailed to the
 ## server administrators.
 # ERROR_REPORTING = True
-## For frontend (JavaScript) tracebacks
-# BROWSER_ERROR_REPORTING = False
 
 ## Controls the DSN used to report errors to Sentry.io
 # SENTRY_DSN = "https://aaa@bbb.ingest.sentry.io/1234"

zproject/urls.py

@@ -132,7 +132,6 @@ from zerver.views.registration import (
 )
 from zerver.views.report import (
     report_csp_violations,
-    report_error,
     report_narrow_times,
     report_send_times,
     report_unnarrow_times,
@@ -491,11 +490,6 @@ v1_api_and_json_patterns = [
     # These endpoints are for internal error/performance reporting
     # from the browser to the web app, and we don't expect to ever
     # include in our API documentation.
-    rest_path(
-        "report/error",
-        # Logged-out browsers can hit this endpoint, for portico page JS exceptions.
-        POST=(report_error, {"allow_anonymous_user_web", "intentionally_undocumented"}),
-    ),
     rest_path("report/send_times", POST=(report_send_times, {"intentionally_undocumented"})),
     rest_path(
         "report/narrow_times",